Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

web: refactor sidebar capabilities for categorical subsections #7482

Merged
merged 24 commits into from
Nov 20, 2023

Conversation

kensternberg-authentik
Copy link
Contributor

Details

web: refactor sidebar capabilities for categorical subsections

The project “Change Admin UI lists to have sublists per type” requires some initial changes to the UI to facilitate this request. The AdminSidebar is the principle target of this project, and it is embedded in the AdminInterface. To facilitate editing the AdminSidebar as an independent entity, AdminInterface has been moved into its own folder and the AdminSidebar extracted as a standalone Web Component. This removes, oh, about half the code from AdminInterface. A little cleanup with classMap was also committed.

The rollup config was adjusted to find the new AdminInterface location.

The Sidebar uses the global config: Config object to check for Enterprise capabilities. Rather than plumb all the way down through the Interface => AdminInterface -> AdminSidebar, I chose to make provide an alternative way of reaching the config object, as a context. Other configuration objects (Me, UiConfig, Tenant) interfaces will be contextualized as demand warrants.

Demand will warrant. Just not yet. 1

The Sidebar has been refactored only slightly; the renderers are entirely the same as they were prior to extraction. What has been changed is the source of information: when we retrieve the current version we story only the information, and use type information to ensure that the version we store is the version we care about. The same is true of impersonation; we care only about the name of the person being impersonated being present, so we don’t store anything else.

Fetches have been moved from firstUpdated to the constructor. No reason to have the sidebar render twice if the network returns before the render is scheduled.

Because the path used to identify the user being impersonated has changed, the str() references in the XLIFF files had to be adjusted. This change is to a variable only and does not require translation.


1 The code is littered with checks to me()?, uiConfig?, config?, etc. In the context of being logged in as an administrator those should never be in doubt. I intend to make our interfaces not have any doubt.


Checklist

  • Local tests pass (ak test authentik/)
  • The code has been formatted (make lint-fix)

If an API change has been made

  • The API schema has been updated (make gen-build)

If changes to the frontend have been made

  • The code has been formatted (make web)
  • The translation files have been updated (make i18n-extract)

If applicable

  • The documentation has been updated
  • The documentation has been formatted (make website)

This commit changes the way the root node of the web application shell is
discovered by child components, such that the base class shared by both
no longer results in a circular dependency between the two models.

I've run this in isolation and have seen no failures of discovery; the identity
token exists as soon as the Interface is constructed and is found by every item
on the page.
* main: (57 commits)
  stages/email: Fix query parameters getting lost in Email links (#5376)
  core/rbac: fix missing field when removing perm, add delete from object page (#7226)
  website/integrations: grafana: add Helm and Terraform config examples (#7121)
  web: bump @types/codemirror from 5.60.11 to 5.60.12 in /web (#7223)
  translate: Updates for file web/xliff/en.xlf in zh_CN (#7224)
  translate: Updates for file web/xliff/en.xlf in zh-Hans (#7225)
  website/blogs: blog about sso tax (#7202)
  web: Application wizard v2 with tests (#7004)
  web: bump API Client version (#7220)
  core: bump goauthentik.io/api/v3 from 3.2023083.7 to 3.2023083.8 (#7221)
  providers/radius: TOTP MFA support (#7217)
  web: bump API Client version (#7218)
  stage/deny: add custom message (#7144)
  docs: update full-dev-setup docs (#7205)
  enterprise: bump license usage task frequency (#7215)
  web: bump the storybook group in /web with 5 updates (#7212)
  web: bump the sentry group in /web with 2 updates (#7211)
  Revert "web: Updates to the Context and Tasks libraries from lit. (#7168)"
  web: bump @types/codemirror from 5.60.10 to 5.60.11 in /web (#7209)
  web: bump @types/chart.js from 2.9.38 to 2.9.39 in /web (#7206)
  ...
* main:
  web/flows: update flow background (#7232)
  web/admin: fix prompt form and codemirror mode (#7231)
  web/admin: decrease wizard hint padding (#7227)
* main:
  web: bump API Client version (#7246)
  sources/oauth: include default JWKS URLs for OAuth sources (#6992)
  sources/oauth: periodically update OAuth sources' OIDC configuration (#7245)
  website/blogs: Fix sso blog to remove 3rd reason (#7230)
  lifecycle: fix otp_merge migration again (#7244)
  web: bump core-js from 3.33.0 to 3.33.1 in /web (#7243)
  core: bump node from 20 to 21 (#7237)
  web: fix bad comment that was confusing lit-analyze (#7234)
  translate: Updates for file web/xliff/en.xlf in zh_CN (#7235)
  core: bump ruff from 0.1.0 to 0.1.1 (#7238)
  core: bump twilio from 8.9.1 to 8.10.0 (#7239)
  web: bump the storybook group in /web with 5 updates (#7240)
  web: bump the wdio group in /tests/wdio with 4 updates (#7241)
  translate: Updates for file web/xliff/en.xlf in zh-Hans (#7236)
  web: isolate clipboard handling (#7229)
This built... and then it didn't?  Anyway, the current fix is to
provide type information the AkInterface for the data that consumers
require.
* main:
  sources/oauth: fix oidc well-known parsing (#7248)
  web/admin: improve user email button labels (#7233)
* main:
  core: bump pylint-django from 2.5.3 to 2.5.4 (#7255)
  core: bump goauthentik.io/api/v3 from 3.2023083.9 to 3.2023083.10 (#7256)
  web: bump the wdio group in /tests/wdio with 1 update (#7258)
  web: bump the eslint group in /tests/wdio with 1 update (#7257)
  sources/oauth: fix name clash (#7253)
  web: bump the eslint group in /web with 1 update (#7250)
  web: bump mermaid from 10.5.0 to 10.5.1 in /web (#7247)
  web: break circular dependency between AKElement & Interface. (#7165)
* main: (28 commits)
  web: fix typo in traefik name
  web/admin: disable wizard banner for now (#7294)
  web/admin: small fixes (#7292)
  core: Use branding_title in the end session page (#7282)
  web: bump pyright from 1.1.332 to 1.1.333 in /web (#7287)
  website: bump react-tooltip from 5.21.5 to 5.21.6 in /website (#7283)
  web: bump the sentry group in /web with 2 updates (#7285)
  web: bump the eslint group in /web with 1 update (#7286)
  core: bump ruff from 0.1.1 to 0.1.2 (#7289)
  core: bump pytest from 7.4.2 to 7.4.3 (#7288)
  web: bump the wdio group in /tests/wdio with 3 updates (#7290)
  website/blogs: fixed typo in blog (#7281)
  core: bump pylint from 2.17.7 to 3.0.2 (#7270)
  web: bump the eslint group in /tests/wdio with 2 updates (#7274)
  translate: Updates for file web/xliff/en.xlf in zh-Hans (#7278)
  translate: Updates for file web/xliff/en.xlf in zh_CN (#7277)
  ci: bump actions/setup-node from 3 to 4 (#7268)
  core: bump pylint-django from 2.5.4 to 2.5.5 (#7271)
  web: bump the eslint group in /web with 2 updates (#7269)
  web: bump @trivago/prettier-plugin-sort-imports from 4.2.0 to 4.2.1 in /tests/wdio (#7275)
  ...
* main: (38 commits)
  crypto: fix race conditions when creating self-signed certificates on startup (#7344)
  blueprints: fix entries with state: absent not being deleted if their serializer has errors (#7345)
  web/admin: fix @change handler for ak-radio elements (#7348)
  rbac: handle lookup error (#7341)
  website/docs: add warning about upgrading to 2023.10 (#7340)
  web/admin: fix role form reacting to enter (#7330)
  core: bump github.com/google/uuid from 1.3.1 to 1.4.0 (#7333)
  core: bump goauthentik.io/api/v3 from 3.2023083.10 to 3.2023101.1 (#7334)
  core: bump ruff from 0.1.2 to 0.1.3 (#7335)
  core: bump pydantic-scim from 0.0.7 to 0.0.8 (#7336)
  website/blogs: Blog dockers (#7328)
  providers/proxy: attempt to fix duplicate cookie (#7324)
  stages/email: fix sending emails from task (#7325)
  web: bump API Client version (#7321)
  website/docs: update release notes for 2023.10.1 (#7316)
  release: 2023.10.1
  lifecycle: fix otp merge migration (#7315)
  root: fix pylint errors (#7312)
  web: bump API Client version (#7311)
  release: 2023.10.0
  ...
* main:
  web: bump rollup from 4.1.4 to 4.1.5 in /web (#7370)
  website/integrations: add SonarQube (#7167)
  web: bump the storybook group in /web with 5 updates (#7382)
  core: bump goauthentik.io/api/v3 from 3.2023101.1 to 3.2023102.1 (#7378)
  web: bump ts-lit-plugin from 2.0.0 to 2.0.1 in /web (#7379)
  web: bump @rollup/plugin-replace from 5.0.4 to 5.0.5 in /web (#7380)
  web: bump API Client version (#7365)
  website/docs: add 2023.8.4 release notes
  release: 2023.10.2
  security: fix oobe-flow reuse when akadmin is deleted (#7361)
  website/docs: prepare 2023.10.2 release notes (#7362)
  website/docs: add missing breaking change due to APPEND_SLASH (#7360)
  lifecycle: rework otp_merge migration (#7359)
  translate: Updates for file web/xliff/en.xlf in zh-Hans (#7354)
  translate: Updates for file web/xliff/en.xlf in zh_CN (#7353)
  website/docs: add warning about Helm breaking change in 2024.x (#7351)
* main:
  website/integrations: argocd: add missing url in ArgoCD configuration (#7404)
  core: bump sentry-sdk from 1.32.0 to 1.33.1 (#7397)
  core: bump webauthn from 1.11.0 to 1.11.1 (#7399)
  core: bump github.com/redis/go-redis/v9 from 9.2.1 to 9.3.0 (#7396)
  core: bump twisted from 23.8.0 to 23.10.0 (#7398)
  web: bump the sentry group in /web with 2 updates (#7401)
  web: bump pyright from 1.1.333 to 1.1.334 in /web (#7402)
  web: bump rollup from 4.1.5 to 4.2.0 in /web (#7403)
  core: bump pytest-django from 4.5.2 to 4.6.0 (#7387)
  web: bump the eslint group in /tests/wdio with 2 updates (#7388)
  web: bump the sentry group in /web with 2 updates (#7366)
  web: bump the eslint group in /web with 2 updates (#7389)
  web: bump core-js from 3.33.1 to 3.33.2 in /web (#7390)
  stages/email: fix duplicate querystring encoding (#7386)
  web/admin: fix html error on oauth2 provider page (#7384)
* main:
  translate: Updates for file web/xliff/en.xlf in fr (#7416)
  website: bump react-tooltip from 5.21.6 to 5.22.0 in /website (#7412)
  core: bump selenium from 4.14.0 to 4.15.0 (#7411)
  core: bump django from 4.2.6 to 4.2.7 (#7413)
  web: bump the eslint group in /web with 1 update (#7414)
  web: bump the eslint group in /tests/wdio with 1 update (#7415)
  root: Improve multi arch Docker image build speed (#7355)
* main:
  providers/proxy: fix closed redis client (#7385)
  ci: explicitly give write permissions to packages (#7428)
  core: bump selenium from 4.15.0 to 4.15.1 (#7422)
  web: bump yaml from 2.3.3 to 2.3.4 in /web (#7420)
  core: bump sentry-sdk from 1.33.1 to 1.34.0 (#7421)
  web: bump the wdio group in /tests/wdio with 4 updates (#7423)
  providers/oauth2: set auth_via for token and other endpoints (#7417)
  website/blog: draft for happy bday blog (#7408)
* main:
  web/flows: attempt to fix bitwareden android compatibility (#7455)
  sources/oauth: fix patreon (#7454)
  website: bump the docusaurus group in /website with 3 updates (#7400)
  web/admin: fix chart label on dashboard user page (#7434)
  core: bump github.com/gorilla/sessions from 1.2.1 to 1.2.2 (#7446)
  core: bump github.com/gorilla/mux from 1.8.0 to 1.8.1 (#7443)
  core: bump github.com/spf13/cobra from 1.7.0 to 1.8.0 (#7442)
  core: bump github.com/gorilla/websocket from 1.5.0 to 1.5.1 (#7445)
  core: bump golang.org/x/sync from 0.4.0 to 0.5.0 (#7441)
  core: bump github.com/gorilla/securecookie from 1.1.1 to 1.1.2 (#7440)
  core: bump github.com/gorilla/handlers from 1.5.1 to 1.5.2 (#7444)
  web: bump rollup from 4.2.0 to 4.3.0 in /web (#7448)
  web: bump the eslint group in /web with 2 updates (#7447)
  core: bump uvicorn from 0.23.2 to 0.24.0 (#7450)
  core: bump selenium from 4.15.1 to 4.15.2 (#7449)
  core: bump ruff from 0.1.3 to 0.1.4 (#7451)
  web: bump the eslint group in /tests/wdio with 2 updates (#7452)
* main:
  web: bump @types/chart.js from 2.9.39 to 2.9.40 in /web
  website/integrations: add FreshRSS (#7301)
  web: bump the eslint group in /web with 2 updates
  core: bump uvicorn from 0.24.0 to 0.24.0.post1
  web: bump the storybook group in /web with 5 updates
  web: bump the eslint group in /tests/wdio with 2 updates
  web: bump @types/codemirror from 5.60.12 to 5.60.13 in /web
  web: bump mermaid from 10.6.0 to 10.6.1 in /web
  translate: Updates for file web/xliff/en.xlf in fr (#7461)
  translate: Updates for file web/xliff/en.xlf in zh-Hans (#7459)
  translate: Updates for file web/xliff/en.xlf in zh_CN (#7458)
  web: bump @lit/localize-tools from 0.7.0 to 0.7.1 in /web (#7369)
  web: bump @lit-labs/context from 0.4.1 to 0.5.1 in /web (#7368)
The most frustrating part of this is that I RAN THIS, dammit, with the updated
context and the current Wizard, and it finished the End-to-End tests without
complaint.
* main:
  Web: bugfix: broken backchannel selector (#7480)
  web: rollback dependabot context (#7479)
This reverts commit 829ad5d.
The project "Change Admin UI lists to have sublists per type" requires some initial changes to the
UI to facilitate this request. The AdminSidebar is the principle target of this project, and it is
embedded in the AdminInterface. To facilitate editing the AdminSidebar as an independent entity,
AdminInterface has been moved into its own folder and the AdminSidebar extracted as a standalone Web
Component. This removes, oh, about half the code from AdminInterface. A little cleanup with
`classMap` was also committed.

The rollup config was adjusted to find the new AdminInterface location.

The Sidebar uses the global `config: Config` object to check for Enterprise capabilities. Rather
than plumb all the way down through the Interface => AdminInterface -> AdminSidebar, I chose to make
provide an alternative way of reaching the `config` object, as a *context*. Other configuration
objects (Me, UiConfig, Tenant) interfaces will be contextualized as demand warrants.

Demand will warrant.  Just not yet. <sup>1</sup>

The Sidebar has been refactored only slightly; the renderers are entirely the same as they were
prior to extraction. What has been changed is the source of information: when we retrieve the
current version we story *only* the information, and use type information to ensure that the version
we store is the version we care about. The same is true of `impersonation`; we care only about the
name of the person being impersonated being present, so we don't store anything else.

Fetches have been moved from `firstUpdated` to the constructor.  No reason to have the sidebar
render twice if the network returns before the render is scheduled.

Because the path used to identify the user being impersonated has changed, the `str()` references in
the XLIFF files had to be adjusted. **This change is to a variable only and does not require
translation.**

---
<sup>1</sup> The code is littered with checks to `me()?`, `uiConfig?`, `config?`, etc. In the
*context* of being logged in as an administrator those should never be in doubt. I intend to make
our interfaces not have any doubt.
Copy link

netlify bot commented Nov 7, 2023

Deploy Preview for authentik-storybook ready!

Name Link
🔨 Latest commit 476adef
🔍 Latest deploy log https://app.netlify.com/sites/authentik-storybook/deploys/6553bbc3ad0c76000851c435
😎 Deploy Preview https://deploy-preview-7482--authentik-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -148,7 +148,7 @@ export default [
},
// Admin interface
{
input: "./src/admin/AdminInterface.ts",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjust the location for the Interface root, since we're moving it.

}

render(): TemplateResult {
const sidebarClasses = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the Sidebar-ectomy, the only other changes are the removal of imports used only by the Sidebar code, and this update to the styles to make the HTML a little cleaner.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we maybe move that logic into the actual sidebar (mainly the collapsing/expanding)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! And I've done just that.

export class AkAdminSidebar extends AKElement {
@property({ type: Boolean })
open = true;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the only bits we care about in either of the calls to AdminApi() and me().

if (this.childItems.length > 0) {
return html`<li
class="pf-c-nav__item ${this.expanded ? "pf-m-expandable pf-m-expanded" : ""}"
renderWithChildren() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The four objects here represent the four main items drawn by the sidebar. We don't yet have the withPathAndChildren() yet; that's for when we get to the subtypes.

@kensternberg-authentik kensternberg-authentik marked this pull request as ready for review November 7, 2023 22:19
@kensternberg-authentik kensternberg-authentik requested a review from a team as a code owner November 7, 2023 22:19
Copy link

codecov bot commented Nov 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dc7ffba) 92.62% compared to head (476adef) 90.74%.
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7482      +/-   ##
==========================================
- Coverage   92.62%   90.74%   -1.88%     
==========================================
  Files         587      587              
  Lines       29029    29029              
==========================================
- Hits        26887    26342     -545     
- Misses       2142     2687     +545     
Flag Coverage Δ
e2e 49.14% <ø> (-1.68%) ⬇️
integration ?
unit 89.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* main: (23 commits)
  web: bump API Client version (#7513)
  web: bump the sentry group in /web with 2 updates (#7500)
  core: bump pytest-django from 4.6.0 to 4.7.0 (#7497)
  core: bump black from 23.10.1 to 23.11.0 (#7498)
  core: bump ruff from 0.1.4 to 0.1.5 (#7499)
  core: bump golang.org/x/oauth2 from 0.13.0 to 0.14.0 (#7501)
  web: bump the wdio group in /tests/wdio with 3 updates (#7502)
  release: 2023.10.3
  ci: fix permissions for release pipeline to publish binaries (#7512)
  website/docs: update release notes for 2023.10.3
  core: fix worker beat toggle inverted (#7508)
  website/docs: update release notes for 2023.10.3
  website/docs: fix anchor link (#7492)
  consistent variable name
  add more tooltips and add device authn/authz
  fix wrong color
  events: fix gdpr compliance always running
  website/docs: Fix a small grammar issue (#7490)
  core: bump golang from 1.21.3-bookworm to 1.21.4-bookworm
  web: bump pyright from 1.1.334 to 1.1.335 in /web
  ...
}

render(): TemplateResult {
const sidebarClasses = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we maybe move that logic into the actual sidebar (mainly the collapsing/expanding)

Move open/close logic into the ak-admin-sidebar itself.

This commit removes the responsibility for opening/closing the sidebar from the interface parent
code and places it inside the sidebar entirely.  Since the Django invocation passes none of the
properties ak-interface-admin is capable of receiving, this seems like a safe operation.

The sidebar now assumes the responsibility for hooking up the window event listeners for open/close
and resize.

On connection to the DOM, and on resize, the sidebar checks to see if the viewport width meets the
criteria for a behavioral change (slide-overlay vs slide-push), and on slide-push automatically
opens the sidebar on the assumption that there's plenty of room. In order to support more dynamic
styling going forward, I've substituted the 1280px with 80rem, which is the same, but allows for
some better styling if someone with older eyes needs to "zoom in" on the whole thing with a larger
font size.

The hide/show code involves "reaching up" to touch the host's classList.  There's a comment
indicating that this is a slightly fragile thing to do, but in a well-known way.
Copy link

netlify bot commented Nov 13, 2023

Deploy Preview for authentik ready!

Name Link
🔨 Latest commit 476adef
🔍 Latest deploy log https://app.netlify.com/sites/authentik/deploys/6553bbc3ce7d800007f9924a
😎 Deploy Preview https://deploy-preview-7482--authentik.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 97 (🟢 up 1 from production)
Accessibility: 90 (no change from production)
Best Practices: 100 (no change from production)
SEO: 80 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

* main: (24 commits)
  internal: remove special route for /outpost.goauthentik.io (#7539)
  providers/proxy: Fix duplicate cookies when using file system store. (#7541)
  web: bump API Client version (#7543)
  sources/ldap: add check command to verify ldap connectivity (#7263)
  internal: remove deprecated metrics (#7540)
  core: compile backend translations (#7538)
  web: bump prettier from 3.0.3 to 3.1.0 in /web (#7528)
  web: bump @trivago/prettier-plugin-sort-imports from 4.2.1 to 4.3.0 in /web (#7531)
  web: bump rollup from 4.3.0 to 4.4.0 in /web (#7529)
  core: bump celery from 5.3.4 to 5.3.5 (#7536)
  web: bump @formatjs/intl-listformat from 7.5.1 to 7.5.2 in /web (#7530)
  web: bump prettier from 3.0.3 to 3.1.0 in /tests/wdio (#7532)
  web: bump @trivago/prettier-plugin-sort-imports from 4.2.1 to 4.3.0 in /tests/wdio (#7533)
  website: bump prettier from 3.0.3 to 3.1.0 in /website (#7534)
  website: bump prism-react-renderer from 2.1.0 to 2.2.0 in /website (#7535)
  translate: Updates for file locale/en/LC_MESSAGES/django.po in zh_TW (#7537)
  root: Restructure broker / cache / channel / result configuration (#7097)
  core: bump twilio from 8.10.0 to 8.10.1 (#7474)
  web: bump axios from 1.5.0 to 1.6.1 in /web (#7518)
  web: bump wdio-wait-for from 3.0.7 to 3.0.8 in /tests/wdio (#7514)
  ...
@kensternberg-authentik kensternberg-authentik merged commit bb52765 into main Nov 20, 2023
58 of 63 checks passed
@kensternberg-authentik kensternberg-authentik deleted the web/sidebar-with-live-content branch November 20, 2023 18:25
kensternberg-authentik added a commit that referenced this pull request Nov 21, 2023
* main:
  web: refactor status label to separate component (#7407)
  web: refactor sidebar capabilities for categorical subsections (#7482)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants