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

Fix search type permissions #5757

Merged
merged 5 commits into from
Oct 16, 2023
Merged

Conversation

distantnative
Copy link
Member

@distantnative distantnative commented Oct 8, 2023

This PR …

Fixes

  • Search now takes access permissions into account what types can be shown

Breaking changes

Ready?

  • Unit tests for fixed bug/feature
  • In-code documentation (wherever needed)
  • Tests and checks all pass

For review team

@distantnative distantnative self-assigned this Oct 8, 2023
@distantnative distantnative mentioned this pull request Oct 8, 2023
5 tasks
@distantnative distantnative added needs: tests 🧪 Requires missing tests and removed needs: tests 🧪 Requires missing tests labels Oct 8, 2023
@distantnative distantnative requested a review from a team October 8, 2023 10:03
@distantnative distantnative marked this pull request as ready for review October 8, 2023 10:03
@distantnative distantnative added this to the 4.0.0-beta.3 milestone Oct 8, 2023
@afbora
Copy link
Member

afbora commented Oct 9, 2023

I can still see the search icon in sidebar when I disabled for all access. Also I see when I clicked to search icon:

permissions:
  access:
    site: false
    users: false
    todos: false

screen-capture-1693-System - Mægazine-localhost

Console

TypeError: Cannot read properties of undefined (reading 'icon')

@afbora
Copy link
Member

afbora commented Oct 9, 2023

Now working correct 👍

One more last minor thing. Disable all types and visit /panel/search. You'll see empty page with following console error:

TypeError: Cannot read properties of undefined (reading 'id')

I know that user can not access to search view. But I feel like we should disable search view something like that https://github.com/getkirby/kirby/pull/5613/files#diff-4eafe8f0737452cffaace81565ddb4cdcf8cd84dfbebf63b39e19346a850b971R10-R16

Disabling search view

screen-capture-1694-Error - Mægazine-localhost

What do you think?

@bastianallgeier
Copy link
Member

I think it's a good idea to disable the search view entirely in that case. @afbora do you want to give it a shot? Or should we wait for @distantnative

@afbora afbora force-pushed the v4/nico/search-permissions branch from aff6053 to a50bd8f Compare October 12, 2023 12:14
@afbora
Copy link
Member

afbora commented Oct 12, 2023

@bastianallgeier I've implemented the disabling entirely search view. I hope it's acceptable for you and Nico.

@afbora afbora force-pushed the v4/nico/search-permissions branch from a50bd8f to d9b9a7e Compare October 13, 2023 11:33
@bastianallgeier bastianallgeier force-pushed the v4/nico/search-permissions branch from d9b9a7e to 78b3841 Compare October 16, 2023 14:03
@bastianallgeier bastianallgeier merged commit 222c297 into v4/develop Oct 16, 2023
@bastianallgeier bastianallgeier deleted the v4/nico/search-permissions branch October 16, 2023 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants