Skip to content

web/user: fix broken search on application library#5743

Merged
kensternberg-authentik merged 1 commit intomainfrom
library-search
May 24, 2023
Merged

web/user: fix broken search on application library#5743
kensternberg-authentik merged 1 commit intomainfrom
library-search

Conversation

@kensternberg-authentik
Copy link
Copy Markdown
Contributor

This is mortifying. I didn't test this well enough, and apparently broke it again once I'd tested it. This patch restores the original behavior ("no match" means "just show everything"), and fixes a small bit of semantic lint -- the "search" feature should not be assigning meaning to what it finds; it's enough to pass back the prioritized list to whatever client wanted it, and let the client decide what to do with it.

Details

  • Does this resolve an issue?

Resolves #5171

Changes

Fixes the search feature so that the data being passed back and forth correspond to the design.

  • 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)

This is *mortifying*.  I didn't test this well enough, and apparently
broke it again once I'd tested it.  This patch restores the original
behavior ("no match" means "just show everything"), and fixes a
small bit of semantic lint -- the "search" feature should not be
assigning meaning to what it finds; it's enough to pass back the
prioritized list to whatever client wanted it, and let the client
decide what to do with it.
@kensternberg-authentik kensternberg-authentik requested a review from a team as a code owner May 24, 2023 15:41

renderSearch() {
return html`<ak-library-list-search .apps="{this.apps.results}"></ak-library-list-search>`;
return html`<ak-library-list-search .apps=${this.apps.results}></ak-library-list-search>`;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was the real issue; everything else is just dealing with errors to the console when the field is cleared (when there is no app[0]). I got this to work, then moved it here and effed-up my argument call. Massive apologies.

@codecov
Copy link
Copy Markdown

codecov bot commented May 24, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (894b4e3) 92.65% compared to head (dfd40e0) 92.64%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5743      +/-   ##
==========================================
- Coverage   92.65%   92.64%   -0.00%     
==========================================
  Files         547      547              
  Lines       26227    26242      +15     
==========================================
+ Hits        24298    24310      +12     
- Misses       1929     1932       +3     
Flag Coverage Δ
e2e 51.96% <ø> (-0.02%) ⬇️
integration 26.42% <ø> (+0.04%) ⬆️
unit 89.54% <ø> (+0.03%) ⬆️

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

see 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link
Copy Markdown
Contributor

authentik PR Installation instructions

Instructions for docker-compose

Add the following block to your .env file:

AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-library-search-1684943529-dfd40e0
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s

For arm64, use these values:

AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-library-search-1684943529-dfd40e0-arm64
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s

Afterwards, run the upgrade commands from the latest release notes.

Instructions for Kubernetes

Add the following block to your values.yml file:

authentik:
    outposts:
        container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
image:
    repository: ghcr.io/goauthentik/dev-server
    tag: gh-library-search-1684943529-dfd40e0

For arm64, use these values:

authentik:
    outposts:
        container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
image:
    repository: ghcr.io/goauthentik/dev-server
    tag: gh-library-search-1684943529-dfd40e0-arm64

Afterwards, run the upgrade commands from the latest release notes.

@BeryJu BeryJu changed the title web: fix broken search on application library web/user: fix broken search on application library May 24, 2023
@kensternberg-authentik kensternberg-authentik merged commit 7c43c1a into main May 24, 2023
@kensternberg-authentik kensternberg-authentik deleted the library-search branch May 24, 2023 18:51
ChandonPierre pushed a commit to ChandonPierre/authentik that referenced this pull request Jun 1, 2023
web: fix broken search on application library

This is *mortifying*.  I didn't test this well enough, and apparently
broke it again once I'd tested it.  This patch restores the original
behavior ("no match" means "just show everything"), and fixes a
small bit of semantic lint -- the "search" feature should not be
assigning meaning to what it finds; it's enough to pass back the
prioritized list to whatever client wanted it, and let the client
decide what to do with it.
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.

UX: add an actionable button to "No Applications" page

2 participants