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

CRDCDH-2187 Submission Request Cancel/Restore #602

Merged
merged 22 commits into from
Feb 5, 2025
Merged

Conversation

amattu2
Copy link
Member

@amattu2 amattu2 commented Jan 22, 2025

Overview

This PR builds out the functionality to Cancel and Restore a Submission Request, and includes supporting QoL updates for related implementations.

Warning

Some implementation notes:

  • The backend refers to the "cancel" action API as "delete", so in the frontend I remapped that to cancelApplication instead of deleteApplication since that's the correct terminology
  • When you delete an application, it disappears from the list. Once we have the filters in place, I assume it would be visible still (if you select the Canceled status)

Change Details (Specifics)

  • Add a Cancel/Restore button for the Submission Request List
  • Add associated test coverage
  • Update the GenericTable to mitigate 508 errors when using an empty column header name (Instead of using a TR, switch it to a TD)

Related Ticket(s)

CRDCDH-2187 (Task)
CRDCDH-2146 (US 1)
CRDCDH-2142 (US 2)

@amattu2 amattu2 added the 🚧 Do Not Merge This PR is not ready for merging label Jan 22, 2025
@amattu2 amattu2 added this to the 3.2.0 (PMVP-M3) milestone Jan 22, 2025
@coveralls
Copy link
Collaborator

coveralls commented Jan 23, 2025

Pull Request Test Coverage Report for Build 13159651953

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 52 of 61 (85.25%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 58.537%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/content/questionnaire/ListView.tsx 5 14 35.71%
Totals Coverage Status
Change from base Build 13117373566: 0.4%
Covered Lines: 4111
Relevant Lines: 6593

💛 - Coveralls

@amattu2 amattu2 marked this pull request as ready for review January 23, 2025 16:42
@amattu2 amattu2 removed the 🚧 Do Not Merge This PR is not ready for merging label Jan 24, 2025
Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega left a comment

Choose a reason for hiding this comment

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

Everything looks solid to me. Great work! Also, I agree that the submissions should likely not disappear and Canceled submissions should still be shown.

I left 2 small optional suggestions below. Feel free to disregard.

src/components/CancelApplicationButton/index.tsx Outdated Show resolved Hide resolved
src/config/AuthPermissions.ts Outdated Show resolved Hide resolved
@Alejandro-Vega Alejandro-Vega added the 📝 Change Requested This PR has requested changes label Jan 29, 2025
@amattu2 amattu2 removed the 📝 Change Requested This PR has requested changes label Jan 30, 2025
@amattu2 amattu2 added the 🚧 Do Not Merge This PR is not ready for merging label Jan 31, 2025
@amattu2

This comment was marked as outdated.

@amattu2 amattu2 removed the 🚧 Do Not Merge This PR is not ready for merging label Feb 3, 2025
Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega left a comment

Choose a reason for hiding this comment

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

LGTM! The new tooltips and icons look great!

@Alejandro-Vega Alejandro-Vega merged commit bda826c into 3.2.0 Feb 5, 2025
5 checks passed
@Alejandro-Vega Alejandro-Vega deleted the CRDCDH-2187 branch February 5, 2025 14:33
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.

3 participants