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

Add ability to delete custom analysis methods #12008

Merged
merged 31 commits into from
Jan 21, 2025
Merged

Add ability to delete custom analysis methods #12008

merged 31 commits into from
Jan 21, 2025

Conversation

gmrabian
Copy link
Contributor

@gmrabian gmrabian commented Jan 13, 2025

Description

  • Add analysis methods page to completion calculation
    • Determines if method is part of default, and then applies calculation with the applicable form
      • The calculateEntityCompletion method accepts an array of different forms, but it expects the data to fulfill every form, rather than being able to apply a specific form per dataset, so this was the best way I could see to do it
  • Add logic to disable "submit" button on delete modal for non-end users and when form is locked (submitted)
  • Add delete entity button and modal to DrawerReportPage

Related ticket(s)

CMDCT-4233


How to test

Deployed url

  • Log in as a state user
  • Create and enter a NAAAR
  • Create a plan
  • Go to Analysis Methods
  • Add a custom analysis method
  • Click the X button on the new method
    • Verify it opens the delete modal
    • Verify the text is relevant to analysis methods
    • Verify when you click the submit button in the modal it deletes that method
  • Verify analysis methods completion statusing on the review and submit page when you have completed all methods, including adding custom methods

Navigate to a different drawer type page (in MCPAR you can create a plan and go to plan level indicators)

  • Verify the delete button does not show up in those rows

Notes


Pre-review checklist

  • I have added thorough tests, if necessary
  • I have updated relevant documentation, if necessary
  • I have performed a self-review of my code
  • I have manually tested this PR in the deployed cloud environment

Pre-merge checklist

Review

  • Product: This work has been reviewed and approved by product owner, if necessary

@gmrabian gmrabian added the help wanted Extra attention is needed label Jan 13, 2025
@JonHolman
Copy link
Contributor

When I add multiple, then I hit the delete, from the modal I cannot confirm which one I hit delete on.

@JonHolman
Copy link
Contributor

image The columns are not lining up when I add a bunch with some different frequencies and descriptions.

@karla-vm
Copy link
Collaborator

This looks great! Statusing is working as expected, I was able to add and delete custom methods. One thing I noticed is that when you enter a custom method, the Plans utilizing this method question is not validating correctly, I'm able to save without clicking on a plan checkbox.

In terms of the styling, I played around a bit with adding an empty <Heading /> or a hidden <Button /> and it feels hacky... Your idea of converting this into a table sounds like the best approach for sure! I'll create a follow-up ticket for that refactoring work.

@gmrabian gmrabian removed the help wanted Extra attention is needed label Jan 14, 2025
@gmrabian
Copy link
Contributor Author

When I add multiple, then I hit the delete, from the modal I cannot confirm which one I hit delete on.

@JonHolman this is a good callout! The Figma also does not specify. Perhaps in the future we could add that as an enhancement but for now I am not going to add it to this work.

@gmrabian gmrabian added the ready for review Ready for all the reviews! label Jan 15, 2025
@gmrabian gmrabian marked this pull request as ready for review January 15, 2025 17:03
@gmrabian gmrabian added product review Waiting for product review and removed product review Waiting for product review labels Jan 16, 2025
Copy link
Contributor

@bangbay-bluetiger bangbay-bluetiger left a comment

Choose a reason for hiding this comment

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

Looks good to me! Comments are really nits.

@gmrabian
Copy link
Contributor Author

thanks @bangbay-bluetiger! great feedback as always

Copy link

codeclimate bot commented Jan 17, 2025

Code Climate has analyzed commit 65646f5 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (90% is the threshold).

This pull request will bring the total coverage in the repository to 96.7% (0.0% change).

View more on Code Climate.

@gmrabian gmrabian merged commit 1d5f17a into main Jan 21, 2025
19 checks passed
@gmrabian gmrabian deleted the cmdct-4233 branch January 21, 2025 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Ready for all the reviews!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants