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

P4-2449 Support multiple assessments #1166

Merged
merged 6 commits into from
Nov 26, 2020
Merged

Conversation

cwrw
Copy link
Contributor

@cwrw cwrw commented Nov 25, 2020

Jira link

P4-2449

What?

I have added/removed/altered:

  • Add parent assessment controller and allow PER controller to inherit from it
  • Add assessmentable concern and include in PER model
  • Rename validator and state machine classes to a more generic name

Why?

To add support for the Youth risk assessment, which will initially reuse all logic from the Person Escort Record, extract shared logic into abstract parent classes and concerns.

Serializer reuse will be done in a separate pr.

@teneightfive teneightfive temporarily deployed to bookasecuremove-api-pr-1166 November 25, 2020 10:08 Inactive
@cwrw cwrw changed the title P4-2449 Support multiple frameworks P4-2449 Support multiple assessments Nov 25, 2020
@cwrw cwrw force-pushed the P4-2449-support-multiple-frameworks branch from 5753b07 to db6d897 Compare November 26, 2020 09:24
@teneightfive teneightfive temporarily deployed to bookasecuremove-api-pr-1166 November 26, 2020 09:24 Inactive
@cwrw cwrw force-pushed the P4-2449-support-multiple-frameworks branch from db6d897 to 79ca230 Compare November 26, 2020 09:41
@teneightfive teneightfive temporarily deployed to bookasecuremove-api-pr-1166 November 26, 2020 09:41 Inactive
Copy link
Contributor

@smoothcontract smoothcontract left a comment

Choose a reason for hiding this comment

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

Really nice refactor, just one minor niggle which I'm happy to discuss if you don't agree with - don't have a strong opinion around that 🥇

spec/support/a_framework_assessment.rb Outdated Show resolved Hide resolved
jira-ticket: https://dsdmoj.atlassian.net/browse/P4-2449

To allow support for multiple assessments, move shared logic to an
abstract controller for "assessmentables". Define the specific class
in the specific controller for the assessment.

Also move param validator to assessmentable service.
jira-ticket: https://dsdmoj.atlassian.net/browse/P4-2449

Rename contants to generic assessment for person escort
record status enums.
jira-ticket: https://dsdmoj.atlassian.net/browse/P4-2449

* For reuse across multiple assessments, move PER model logic to a
concern, and move tests to a shared example.

* Rename state machine to a generic framework assessment name
* Ignore rubocop warning as its a false positive
jira-ticket: https://dsdmoj.atlassian.net/browse/P4-2449

Move prefill belongs_to to per model, also cleanup unnecessary
memoization.
@cwrw cwrw force-pushed the P4-2449-support-multiple-frameworks branch from 79ca230 to 65c44ae Compare November 26, 2020 11:23
@teneightfive teneightfive temporarily deployed to bookasecuremove-api-pr-1166 November 26, 2020 11:23 Inactive
@teneightfive teneightfive temporarily deployed to bookasecuremove-api-pr-1166 November 26, 2020 11:29 Inactive
Copy link
Contributor

@smoothcontract smoothcontract left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@cwrw cwrw merged commit 1f82c4c into master Nov 26, 2020
@cwrw cwrw deleted the P4-2449-support-multiple-frameworks branch November 26, 2020 11:34
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.

4 participants