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.

Sign up for GitHub

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

Jump to bottom

EREGCSC-2113 -- Reflect edited statute dates in frontend #929

Merged
merged 15 commits into from
Aug 16, 2023
Merged

EREGCSC-2113 -- Reflect edited statute dates in frontend #929

merged 15 commits into from
Aug 16, 2023

Conversation

PhilR8
Copy link
Contributor

@PhilR8 PhilR8 commented Aug 11, 2023 •
edited
Loading

Resolves EREGCSC-2113

Description:

Pulls dates from site configuration in admin panel into front end for Statute Table column header labels.

This pull request changes:

  • Adds statute table site configuration labels and dates into Statutes template using best practice
  • Only shows dates if both label and date exist, and if date is a valid date
  • Updates snapshot tests and adds unit front end unit test coverage

Steps to manually verify this change:

  1. Go into admin panel for experimental deployment > Regulations > Site Configuration
  2. Edit Date Type and Date
  3. Try out a bunch of options. If Date Type and Date are both valid, the date subtitle should appear in the column header. If one or both of the Date Type and Date are empty strings or invalid, the date subtitle will not appear.

@PhilR8 PhilR8 temporarily deployed to dev August 11, 2023 18:40 — with GitHub Actions Inactive
@github-actions
Copy link

✨ See the Django Site in action ✨

@PhilR8 PhilR8 temporarily deployed to dev August 11, 2023 19:14 — with GitHub Actions Inactive
@github-actions
Copy link

✨ See the Django Site in action ✨

@PhilR8 PhilR8 temporarily deployed to dev August 11, 2023 19:32 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 11, 2023 19:35 — with GitHub Actions Inactive
@github-actions
Copy link

✨ See the Django Site in action ✨

@PhilR8 PhilR8 temporarily deployed to dev August 11, 2023 19:51 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 11, 2023 19:54 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 11, 2023 19:58 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 11, 2023 19:58 — with GitHub Actions Inactive
@github-actions
Copy link

✨ See the Django Site in action ✨

@PhilR8 PhilR8 temporarily deployed to dev August 14, 2023 17:26 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 14, 2023 17:29 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 14, 2023 17:33 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 14, 2023 17:33 — with GitHub Actions Inactive
@github-actions
Copy link

✨ See the Django Site in action ✨

@PhilR8 PhilR8 temporarily deployed to dev August 14, 2023 18:41 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 14, 2023 18:49 — with GitHub Actions Inactive
@github-actions
Copy link

✨ See the Django Site in action ✨

PhilR8
PhilR8 commented Aug 14, 2023
View reviewed changes
solution/backend/regulations/views/statute.py
'type': site_config.ssa_gov_compilation_date_type,
'date': site_config.ssa_gov_compilation_date,
},
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Emulated the pattern used in reader.py view to bring in site config values to use in template

Copy link
Contributor

Choose a reason for hiding this comment

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

Not really a fan of this implementation of a singleton model. We are querying SiteConfiguration and storing into site_config. Then we are redefining site_config with the values of site_config and storing it into an object that has 4 fields that have identical properties. This could easily just be a queryset of 4 objects that already have these defined properties.

@PhilR8 PhilR8 temporarily deployed to dev August 14, 2023 19:06 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 14, 2023 19:10 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 14, 2023 19:16 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 14, 2023 19:16 — with GitHub Actions Inactive
@github-actions
Copy link

✨ See the Django Site in action ✨

@PhilR8 PhilR8 marked this pull request as ready for review August 14, 2023 19:31
thwalker6
thwalker6 approved these changes Aug 15, 2023
View reviewed changes
Copy link
Contributor

@thwalker6 thwalker6 left a comment

Choose a reason for hiding this comment

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

it looks good. Just left notes here for discussion as a team. LGTM

@PhilR8 PhilR8 temporarily deployed to dev August 15, 2023 14:23 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 15, 2023 14:26 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 15, 2023 14:30 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 15, 2023 14:30 — with GitHub Actions Inactive
@github-actions
Copy link

✨ See the Django Site in action ✨

@PhilR8 PhilR8 temporarily deployed to dev August 15, 2023 19:47 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 15, 2023 19:50 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 15, 2023 19:54 — with GitHub Actions Inactive
@PhilR8 PhilR8 temporarily deployed to dev August 15, 2023 19:54 — with GitHub Actions Inactive
@github-actions
Copy link

✨ See the Django Site in action ✨

@PhilR8 PhilR8 merged commit cf1c5e0 into main Aug 16, 2023
16 checks passed
@PhilR8 PhilR8 temporarily deployed to dev August 16, 2023 12:10 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thwalker6 thwalker6 thwalker6 approved these changes

@cgodwin1 cgodwin1 Awaiting requested review from cgodwin1 cgodwin1 is a code owner

@peggles2 peggles2 Awaiting requested review from peggles2

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@PhilR8 @thwalker6