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

Member codes docs #2595

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

Member codes docs #2595

wants to merge 5 commits into from

Conversation

elichad
Copy link
Contributor

@elichad elichad commented Dec 5, 2023

Addresses #2576 except for the admin guide updates.

Merge when ready to release v4.3.

@elichad elichad added this to the v4.3 milestone Dec 5, 2023
@elichad elichad added this to In Progress in Member code enforcement via automation Dec 5, 2023
1. Add a field `<fieldname>-override` to the relevant model. This should be a `BooleanField` defaulting to `False`.
2. On `Form`s connected to the model, manipulate the `helper.layout` to show/hide the new field according to the value of the field you're implementing soft validation on (e.g. the `validate_member_code` method of `TrainingRequestForm` in [`/extforms/forms.py`](../../amy/extforms/forms.py) shows and hides the `member_code_override` field according to the validity of the `member_code`).
3. Build validation carefully for the override and the field it relates to. The override should only be required and `True` if the related field is invalid. In other cases, it should be `False` - this may require updating the value during validation (e.g. the `validate_member_code` method again).
4. Consider adding a filter to help admins find objects where the override was used. Beware that the default `django_filters.BooleanFilter` is not quite appropriate - typically you will want *all* results to be shown when the filter is `False`, and only results that use the override to be shown when the filter is `True` (e.g. `invalid_member_code` filter in `TrainingRequestFilter` in [/extrequests/filters.py](../../amy/extrequests/filters.py)).
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider absolute link to the file in GitHub repository.

**Method**:

1. Add a field `<fieldname>-override` to the relevant model. This should be a `BooleanField` defaulting to `False`.
2. On `Form`s connected to the model, manipulate the `helper.layout` to show/hide the new field according to the value of the field you're implementing soft validation on (e.g. the `validate_member_code` method of `TrainingRequestForm` in [`/extforms/forms.py`](../../amy/extforms/forms.py) shows and hides the `member_code_override` field according to the validity of the `member_code`).
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider absolute link to the file in GitHub repository.


## Feature flag

The `ENFORCE_MEMBER_CODES` [feature flag](../feature_flags.md) controls whether validity checks are performed on member codes. If set to `True` in the Django admin panel, validity checks will be performed on all submissions. If set to `False`, no checks will be performed (i.e. all codes will be accepted). The value of the flag does **not** affect whether the `member_code` field is displayed on forms or in views.
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to switch the feature flag ENFORCE_MEMBER_CODES on before release 4.3.

Member code enforcement automation moved this from In Progress to In Review Dec 11, 2023
@pbanaszkiewicz
Copy link
Contributor

Approved. This should be merged before releasing 4.3. Some additional changes were mentioned during the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants