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

Streamline defining and usage of preferences #1072

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

Conversation

elfjes
Copy link
Collaborator

@elfjes elfjes commented Dec 9, 2024

  • Render all preferences at page load instead of through ajax requests
  • Funnel all preferences update requests through a single view/endpoint
  • Introduces a dataclass to hold preference meta information so that this can be stored directly in the Preference class:
@dataclasses.dataclass
class PreferenceField:
    form: forms.Form
    choices: Optional[Sequence] = None
    default: Optional[Any] = None
    partial_response_template: Optional[str] = None

@elfjes elfjes marked this pull request as draft December 9, 2024 14:34
@elfjes elfjes marked this pull request as ready for review December 10, 2024 13:09
@codecov-commenter
Copy link

codecov-commenter commented Dec 10, 2024

Codecov Report

Attention: Patch coverage is 64.28571% with 25 lines in your changes missing coverage. Please review.

Project coverage is 80.95%. Comparing base (9aa7f15) to head (3f747a4).

Files with missing lines Patch % Lines
src/argus/auth/utils.py 52.38% 10 Missing ⚠️
src/argus/htmx/user/views.py 33.33% 10 Missing ⚠️
src/argus/auth/models.py 83.33% 4 Missing ⚠️
src/argus/auth/context_processors.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1072      +/-   ##
==========================================
- Coverage   81.08%   80.95%   -0.14%     
==========================================
  Files         141      137       -4     
  Lines        5087     5066      -21     
==========================================
- Hits         4125     4101      -24     
- Misses        962      965       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

preferences_choices = {}
for namespace, cls in Preferences.NAMESPACES.items():
preferences_choices[namespace] = {
name: field.choices for name, field in cls.FIELDS.items() if field.choices is not None
Copy link
Contributor

Choose a reason for hiding this comment

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

You're assuming all preferences forever will be "select from dropdown".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope, I'm just saying that if they do have choices, you can add them in the PreferenceField directly and they will become available in the rendering context, to be used however the developer sees fit

@hmpf hmpf requested a review from a team December 11, 2024 09:31
@hmpf hmpf added frontend Affects frontend performance labels Dec 11, 2024
@elfjes elfjes changed the title Simplify preferences Streamline defining and usage of preferences Dec 11, 2024
Copy link

sonarcloud bot commented Dec 11, 2024

@elfjes elfjes requested a review from hmpf December 12, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Ready for review
Development

Successfully merging this pull request may close these issues.

3 participants