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

Filter system host when is isRequired #28931

Conversation

nicobytes
Copy link
Contributor

@nicobytes nicobytes commented Jun 18, 2024

If a host/folder field is marked as required, then we do not allow/show SYSTEM_HOST

Proposed Changes

  • Add isRequired attr to enable filter

Checklist

  • Tests
  • Translations
  • Security Implications Contemplated (add notes if applicable)

This PR fixes: #28924

…equired-then-we-do-not-allowshow-system_host
@nicobytes nicobytes requested a review from zJaaal June 18, 2024 21:10
@nicobytes nicobytes requested a review from zJaaal June 18, 2024 21:21
…equired-then-we-do-not-allowshow-system_host
@fmontes
Copy link
Member

fmontes commented Jun 19, 2024

Are we sure this is the right approach? Because why required will exclude the system host?

What about load without a value and if is marked as required user will have to add a value and that value could be system host.

If we don't want the user to select system host we can do something else? Blacklist maybe?

I don't know all the details behind this reasoning, but this is a component we might be using in a lot of places.

@dsilvam
Copy link
Contributor

dsilvam commented Jun 19, 2024

@fmontes RE: don't show system host if required:
tagged you on Slack in the thread were this was discussed.

…equired-then-we-do-not-allowshow-system_host
@nicobytes nicobytes enabled auto-merge June 19, 2024 13:00
@nicobytes nicobytes added this pull request to the merge queue Jun 19, 2024
Merged via the queue into master with commit 5987d22 Jun 19, 2024
16 checks passed
@nicobytes nicobytes deleted the 28924-if-a-hostfolder-field-is-marked-as-required-then-we-do-not-allowshow-system_host branch June 19, 2024 14:59
oidacra pushed a commit that referenced this pull request Jun 26, 2024
If a host/folder field is marked as required, then we do not allow/show
SYSTEM_HOST

### Proposed Changes
* Add isRequired attr to enable filter

### Checklist
- [x] Tests
- [x] Translations
- [x] Security Implications Contemplated (add notes if applicable)
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.

if a host/folder field is marked as required, then we do not allow/show SYSTEM_HOST
5 participants