web: clean up UserInterface in prep for OAuth and Silo Projects by kensternberg-authentik · Pull Request #8278 · goauthentik/authentik

kensternberg-authentik · 2024-01-23T23:04:54Z

While looking through @BeryJu's Oauth-for-authentik stuff, changes to UserInterface triggered the "harder" eslint pass, which said "UserInterface exceeds permitted complexity (9)." I couldn't disagree; it had lots of conditionals.

This commit:

Changes no functionality; it's just cleanup.
Breaks UserInterface into business and presentation layers
The presentation layer:
- Further breaks the presentation into a frame and conditional components. Each condition is now proceeded by a simple guard clause.
- Taps into the event listener set-up for toggles, eliminating their local scope/window duplication
- Extracts in-line complex expressions into isolated and scoped functions to name them and make them easier to find and read.
- Extracts the custom CSS into its own named variable, again, making it easier to find and read.
The business layer:
- Builds the window-level event listeners at connection, and disconnects them correctly, allowing this whole interface to be used in a SPA. (Not as a SPA, but in a SPA!)
- Asserts a reliable contract at the presentation layer; there should be no question "Session" and "UIConfig" are available before rendering.
- Renames firstUpdated to fetchConfigurationDetails, and calls it in the constructor. There ought to be no circumstances where this object is constructed outside a working environment; no sense in waiting until you've done a render() { nothing } pass to fetch details.

Oddities: There are a pair of  HTML comments in the framing render(); those are there just to stop prettier from slamming a string of conditional renders all into one line, making them harder to read.

Checklist

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

If an API change has been made

[N] The API schema has been updated (make gen-build)

If changes to the frontend have been made

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

If applicable

[N] The documentation has been updated
[N] The documentation has been formatted (make website)

… to the UserInterface triggered the "harder" eslint pass, which said "UserInterface exceeds permitted complexity (9)." I couldn't disagree; it had lots of conditionals. This commit: - Changes no functionality; it's just cleanup. - Breaks UserInterface into business and presentation layers - The presentation layer: - Further breaks the presentation layer into a frame and conditional components. Each conditional is now a simple guard condition. - Taps into the event listener set-up for toggles, eliminating their local scope/window duplication - Extracts in-line complex expressions into isolated and scope functions to name them and make them easier to find and read. - Extracts the custom CSS into its own named variable, again, making it easier to find and read. - The business layer: - Builds the window-level event listener at connection, and disconnects them correctly, allowing this whole interface to be used in a SPA. - Asserts a reliable contract at the presentation layer; there should be no question "Session" and "UIConfig" are available before rendering. - Renames `firstUpdated` to `fetchConfigurationDetails`, and calls it in the constructor. There ought to be no circumstances where this object is constructed outside a working environment; no sense in waiting until you've done a `render() { nothing }` pass to fetch details. Oddities: There are a pair of `` HTML comments in the framing `render()`; those are there just to stop prettier from slamming a string of conditional renders all into one line, making them harder to read.

netlify · 2024-01-23T23:04:59Z

✅ Deploy Preview for authentik-storybook ready!

Name	Link
🔨 Latest commit	`29fe950`
🔍 Latest deploy log	https://app.netlify.com/sites/authentik-storybook/deploys/65e602511e27ed00088c8026
😎 Deploy Preview	https://deploy-preview-8278--authentik-storybook.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

kensternberg-authentik

Added the guided tour.

kensternberg-authentik · 2024-01-23T23:06:04Z

web/src/user/UserInterface.ts

+    @property({ type: Object })
+    brand!: CurrentBrand;
+
+    get userDisplayName() {


Turns this into a switch expression, so Typescript can type it and assert conditional exhaustion correctly!

kensternberg-authentik · 2024-01-23T23:06:40Z

web/src/user/UserInterface.ts

-                                  </div>`
-                                : html``}
+                            ${this.renderApiDrawer()}
+                            <!-- -->


These are here just to space things out; otherwise, prettier wants to string all these together into a single line, which was harder to read.

kensternberg-authentik · 2024-01-23T23:07:47Z

web/src/user/UserInterface.ts

+            return nothing;
+        }
+
+        const onClick = (ev: Event) => {


The current version has two handlers for EVENT_API_DRAWER_TOGGLE, one attached to UserInterface, another attached to window. No sense in keeping both; this eliminates one of them.

would probably rename this to renderApiDrawerTrigger since it doesn't render the actual drawer

kensternberg-authentik · 2024-01-23T23:09:21Z

web/src/user/UserInterface.ts

+        this.ws = new WebsocketClient();
+        this.fetchConfigurationDetails();
+        configureSentry(true);
+        this.toggleNotificationDrawer = this.toggleNotificationDrawer.bind(this);


All this binding is annoying, but necessary; it allows us to connect and disconnect these handlers from window at need. Which would also allow us, if we wanted, to switch between "User" and "Admin" apps without actually leaving the page or reloading the environment, since we're now cleaning up after ourselves.

Switching between all these interfaces without a full reload would definitely be very nice to have

kensternberg-authentik · 2024-01-23T23:10:01Z

web/src/user/UserInterface.ts

+        if (!this.isFullyConfigured) {
+            return nothing;
+        }
+


I like this because it makes it very clear what the presentation responds to: here are the state variables that will trigger a re-render.

kensternberg-authentik · 2024-01-23T23:11:28Z

web/src/user/UserInterface.ts

-                                      <button
-                                          class="pf-c-button pf-m-plain"
-                                          type="button"
-                                          @click=${() => {


This duplicates the functionality of the window.EVENT_API_DRAWER_TOGGLE functionality above, so replacing this with "send the EVENTS_API_DRAWER_TOGGLE event to window" eliminates that duplication.

codecov · 2024-01-23T23:13:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.20%. Comparing base (cef1d2d) to head (c7a0f77).
Report is 256 commits behind head on main.

❗ Current head c7a0f77 differs from pull request most recent head 29fe950. Consider uploading reports for the commit 29fe950 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #8278       +/-   ##
===========================================
+ Coverage   46.62%   92.20%   +45.58%     
===========================================
  Files         626      626               
  Lines       30996    30922       -74     
===========================================
+ Hits        14451    28511    +14060     
+ Misses      16545     2411    -14134

Flag	Coverage Δ
e2e	`49.42% <ø> (+4.70%)`	⬆️
integration	`26.02% <ø> (+0.03%)`	⬆️
unit	`89.58% <ø> (?)`

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: (30 commits) web: clear out selecteds list after an API event to ensure a fresh copy of the policies-to-delete list (#8125) web: provide dual-list multiselect with pagination (#8004) web: provide a context for checking the status of the enterprise license (#8153) core: compile backend translations (#8311) translate: Updates for file web/xliff/en.xlf in zh-Hans (#8304) translate: Updates for file web/xliff/en.xlf in zh_CN (#8305) translate: Updates for file locale/en/LC_MESSAGES/django.po in zh_CN (#8300) translate: Updates for file locale/en/LC_MESSAGES/django.po in zh-Hans (#8301) events: fix missing labels on prometheus metrics (#8309) core: bump goauthentik.io/api/v3 from 3.2023106.4 to 3.2023106.5 (#8302) web: bump the wdio group in /tests/wdio with 4 updates (#8303) web: restore test anchor tag (#8298) translate: Updates for file web/xliff/en.xlf in fr (#8296) translate: Updates for file locale/en/LC_MESSAGES/django.po in fr (#8295) website: update wording (#8290) enterrpise: exclude inactive users from license (#8294) web: bump API Client version (#8292) core: compile backend translations (#8291) events: migrate SystemTasks to DB (#8159) web/admin: fix footer links not being parsed on settings page (#8289) ...

BeryJu

LGTM

BeryJu · 2024-01-26T16:36:56Z

web/src/user/UserInterface.ts

+            return nothing;
+        }
+
+        const onClick = (ev: Event) => {


would probably rename this to renderApiDrawerTrigger since it doesn't render the actual drawer

BeryJu · 2024-01-26T16:37:04Z

web/src/user/UserInterface.ts

+        </div>`;
+    }
+
+    renderNotificationDrawer() {


Same as above

BeryJu · 2024-01-26T16:37:29Z

web/src/user/UserInterface.ts

+        </div>`;
+    }
+
+    renderAccessAdmin() {


Suggested change

renderAccessAdmin() {

renderAdminInterfaceLink() {

BeryJu · 2024-01-26T16:38:02Z

web/src/user/UserInterface.ts

+        this.ws = new WebsocketClient();
+        this.fetchConfigurationDetails();
+        configureSentry(true);
+        this.toggleNotificationDrawer = this.toggleNotificationDrawer.bind(this);


Switching between all these interfaces without a full reload would definitely be very nice to have

* main: (331 commits) website/docs: installation: kubernetes: fix values (#8783) web: bump the wdio group in /tests/wdio with 4 updates (#8789) core: bump github.com/stretchr/testify from 1.8.4 to 1.9.0 (#8790) core: bump twisted from 23.10.0 to 24.3.0 (#8788) translate: Updates for file locale/en/LC_MESSAGES/django.po in zh_CN (#8778) translate: Updates for file locale/en/LC_MESSAGES/django.po in zh-Hans (#8779) root: ensure consistent install_id (#8775) web: bump the sentry group in /web with 1 update (#8762) web: bump style-mod from 4.1.1 to 4.1.2 in /web (#8763) website: bump @types/react from 18.2.60 to 18.2.61 in /website (#8764) core: bump goauthentik.io/api/v3 from 3.2024021.2 to 3.2024021.3 (#8765) core: bump ruff from 0.2.2 to 0.3.0 (#8766) core: bump twilio from 8.13.0 to 9.0.0 (#8767) translate: Updates for file locale/en/LC_MESSAGES/django.po in fr (#8774) core, web: update translations (#8759) web/admin: don't mark LDAP group property mappings as required (#8772) website/docs: move Applications docs up a level, other edits (#8712) web/admin: don't mark property mappings as required anywhere (#8752) website: redirect root to /docs (#8754) web: bump API Client version (#8753) ...

netlify · 2024-03-04T16:18:57Z

✅ Deploy Preview for authentik-docs canceled.

Name	Link
🔨 Latest commit	`29fe950`
🔍 Latest deploy log	https://app.netlify.com/sites/authentik-docs/deploys/65e6025109a69500086017cb

* main: web: clean up UserInterface in prep for OAuth and Silo Projects (#8278) website/docs: installation: kubernetes: fix values (#8783) web: bump the wdio group in /tests/wdio with 4 updates (#8789) core: bump github.com/stretchr/testify from 1.8.4 to 1.9.0 (#8790) core: bump twisted from 23.10.0 to 24.3.0 (#8788) translate: Updates for file locale/en/LC_MESSAGES/django.po in zh_CN (#8778) translate: Updates for file locale/en/LC_MESSAGES/django.po in zh-Hans (#8779) root: ensure consistent install_id (#8775) web: bump the sentry group in /web with 1 update (#8762) web: bump style-mod from 4.1.1 to 4.1.2 in /web (#8763) website: bump @types/react from 18.2.60 to 18.2.61 in /website (#8764) core: bump goauthentik.io/api/v3 from 3.2024021.2 to 3.2024021.3 (#8765) core: bump ruff from 0.2.2 to 0.3.0 (#8766) core: bump twilio from 8.13.0 to 9.0.0 (#8767)

* main: website: fix missing compose file (#8809) core: bump django from 5.0.2 to 5.0.3 (#8808) core: bump github.com/go-openapi/strfmt from 0.22.1 to 0.22.2 (#8801) core, web: update translations (#8800) core: bump goauthentik.io/api/v3 from 3.2024021.3 to 3.2024022.1 (#8802) core: bump golang.org/x/oauth2 from 0.17.0 to 0.18.0 (#8803) core: bump github.com/go-openapi/runtime from 0.27.1 to 0.27.2 (#8804) website: bump @types/react from 18.2.61 to 18.2.62 in /website (#8805) web: bump the eslint group in /tests/wdio with 2 updates (#8806) web: bump the eslint group in /web with 2 updates (#8807) website/integrations: fix typo in proxmox docs (#8791) web: bump API Client version (#8797) release: 2024.2.2 website/docs: prepare 2024.2.2 release notes (#8782) flows: fix mismatched redirect behaviour for invalid and valid flows (#8794) providers/oauth2: fix validation ordering (#8793) web: clean up UserInterface in prep for OAuth and Silo Projects (#8278)

* web/replace-rollup-with-esbuild: website: fix missing compose file (#8809) core: bump django from 5.0.2 to 5.0.3 (#8808) core: bump github.com/go-openapi/strfmt from 0.22.1 to 0.22.2 (#8801) core, web: update translations (#8800) core: bump goauthentik.io/api/v3 from 3.2024021.3 to 3.2024022.1 (#8802) core: bump golang.org/x/oauth2 from 0.17.0 to 0.18.0 (#8803) core: bump github.com/go-openapi/runtime from 0.27.1 to 0.27.2 (#8804) website: bump @types/react from 18.2.61 to 18.2.62 in /website (#8805) web: bump the eslint group in /tests/wdio with 2 updates (#8806) web: bump the eslint group in /web with 2 updates (#8807) website/integrations: fix typo in proxmox docs (#8791) web: bump API Client version (#8797) release: 2024.2.2 website/docs: prepare 2024.2.2 release notes (#8782) flows: fix mismatched redirect behaviour for invalid and valid flows (#8794) providers/oauth2: fix validation ordering (#8793) web: clean up UserInterface in prep for OAuth and Silo Projects (#8278)

kensternberg-authentik commented Jan 23, 2024

View reviewed changes

kensternberg-authentik marked this pull request as ready for review January 23, 2024 23:12

kensternberg-authentik requested a review from a team as a code owner January 23, 2024 23:12

kensternberg-authentik added 2 commits January 23, 2024 15:43

Adding a small experiment: Typescript pattern matching.

9b23fb6

BeryJu approved these changes Jan 26, 2024

View reviewed changes

A few renames as requested by @BeryJu

29fe950

kensternberg-authentik merged commit b059754 into main Mar 4, 2024

kensternberg-authentik deleted the web/clean-user-interface branch March 4, 2024 17:46

BeryJu mentioned this pull request Mar 7, 2024

tests: fix e2e flow tests #8835

Merged

6 tasks

Uh oh!

Conversation

kensternberg-authentik commented Jan 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

netlify bot commented Jan 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for authentik-storybook ready!

Uh oh!

kensternberg-authentik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

BeryJu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

netlify bot commented Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for authentik-docs canceled.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kensternberg-authentik commented Jan 23, 2024 •

edited

Loading

netlify bot commented Jan 23, 2024 •

edited

Loading

codecov bot commented Jan 23, 2024 •

edited

Loading

netlify bot commented Mar 4, 2024 •

edited

Loading