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

Improve HiddenUnless for composite field #6267

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

vtran99
Copy link
Contributor

@vtran99 vtran99 commented Apr 3, 2024

Request change for HiddenUnless validator:
this change renders the validator working on field composed of list of other fields (like "visibility" in Participant List Privacy form),
it also adds an option "inverted" for the validator.

Note:
I added 2 test fields on RegistrationPrivacyForm for visual testing.

@ThiefMaster
Copy link
Member

I added 2 test fields on RegistrationPrivacyForm for visual testing.

Could you split that into a separate commit please? That way it's easier for us to just remove that commit when we are ready to merge it.

Comment on lines 78 to 82
i = 0
while i < len(v):
if values[i] is not None and v[i] not in values[i]:
return False
i += 1
Copy link
Member

Choose a reason for hiding this comment

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

That looks like a good case for something like any(... for ... in zip(v, values))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't see how I can use the zip...
best can do:

if(any(v[i] for i in range(len(v)) if values[i] and v[i] not in values[i]):
  return False

indico/web/forms/validators.py Outdated Show resolved Hide resolved
@vtran99
Copy link
Contributor Author

vtran99 commented Apr 3, 2024

I added 2 test fields on RegistrationPrivacyForm for visual testing.

Could you split that into a separate commit please? That way it's easier for us to just remove that commit when we are ready to merge it.

Done: #6268

@vtran99 vtran99 closed this Apr 3, 2024
@vtran99 vtran99 reopened this Apr 3, 2024
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.

None yet

2 participants