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

1612 case aggregates frontend #2643

Merged
merged 200 commits into from
Oct 5, 2023
Merged

Conversation

jtimpe
Copy link

@jtimpe jtimpe commented Aug 3, 2023

Summary of Changes

Pull request closes #1612

How to Test

List the steps to test the PR
These steps are generic, please adjust as necessary.

cd tdrs-backend && docker-compose up
cd tdrs-frontend && docker-compose up --build
  1. Open http://localhost:3000/ and sign in.
  2. Navigate to Data Files page and submit a data file for the selected reporting month/year
  3. Navigate to Submission History and verify the Month/Cases Without Errors/Cases With Errors/Records Unable to Process columns.

Deliverables

More details on how deliverables herein are assessed included here.

Deliverable 1: Accepted Features

Checklist of ACs:

  • Case data populates table per Figma spec
  • For files with "rejected" or equivalent status, cells show "N/A" and have custom styling
  • Testing Checklist has been run and all tests pass
  • lfrohlich and/or adpennington confirmed that ACs are met.

Deliverable 2: Tested Code

  • Are all areas of code introduced in this PR meaningfully tested?
    • If this PR introduces backend code changes, are they meaningfully tested?
    • If this PR introduces frontend code changes, are they meaningfully tested?
  • Are code coverage minimums met?
    • Frontend coverage: [insert coverage %] (see CodeCov Report comment in PR)
    • Backend coverage: [insert coverage %] (see CodeCov Report comment in PR)

Deliverable 3: Properly Styled Code

  • Are backend code style checks passing on CircleCI?
  • Are frontend code style checks passing on CircleCI?
  • Are code maintainability principles being followed?

Deliverable 4: Accessible

  • Does this PR complete the epic?
  • Are links included to any other gov-approved PRs associated with epic?
  • Does PR include documentation for Raft's a11y review?
  • Did automated and manual testing with iamjolly and ttran-hub using Accessibility Insights reveal any errors introduced in this PR?

Deliverable 5: Deployed

  • Was the code successfully deployed via automated CircleCI process to development on Cloud.gov?

Deliverable 6: Documented

  • Does this PR provide background for why coding decisions were made?
  • If this PR introduces backend code, is that code easy to understand and sufficiently documented, both inline and overall?
  • If this PR introduces frontend code, is that code easy to understand and sufficiently documented, both inline and overall?
  • If this PR introduces dependencies, are their licenses documented?
  • Can reviewer explain and take ownership of these elements presented in this code review?

Deliverable 7: Secure

  • Does the OWASP Scan pass on CircleCI?
  • Do manual code review and manual testing detect any new security issues?
  • If new issues detected, is investigation and/or remediation plan documented?

Deliverable 8: User Research

Research product(s) clearly articulate(s):

  • the purpose of the research
  • methods used to conduct the research
  • who participated in the research
  • what was tested and how
  • impact of research on TDP
  • (if applicable) final design mockups produced for TDP development

andrew-jameson and others added 30 commits April 3, 2023 10:55
- Added admin filter to show newest or all datafile records
- Updated indices to allow easier elastic queries
- Moved create_datafile to util
@jtimpe
Copy link
Author

jtimpe commented Sep 29, 2023

Definitely second fixing the table alignment issue—I dug around and it looks like it's actually a margin problem rather than alignment per se. Removing the margin: 100px on the table div with the class "submission-history-section usa-table-container--scrollable" fixes the issue.

Another a11y issue is a minor error on the blank state of the error reports cell image

That's something we should be filling in with appropriate text when it doesn't have that download button in it, e.g. image

On a broad QA note I don't seem to be getting accurate cases with/without error counts either, and mine also seem to differ from other screenshots in here despite using the same test files. Ex submitted for Indiana Q1 2023 in QASP: small_tanf_section1(1).txt is reading 0s across the board while accepted with errors (on two cases according to the error report).

@reitermb good callouts, thank you! i've removed the offending margin and added in "Pending" and "No Errors" states to the error reports column. Regarding the inconsistent record/error counts - this is something we're aware of and will be discussing, but having unique CASE_NUMBER values for the records in your test file might help

@reitermb
Copy link

reitermb commented Sep 29, 2023

Definitely second fixing the table alignment issue—I dug around and it looks like it's actually a margin problem rather than alignment per se. Removing the margin: 100px on the table div with the class "submission-history-section usa-table-container--scrollable" fixes the issue.
Another a11y issue is a minor error on the blank state of the error reports cell image
That's something we should be filling in with appropriate text when it doesn't have that download button in it, e.g. image
On a broad QA note I don't seem to be getting accurate cases with/without error counts either, and mine also seem to differ from other screenshots in here despite using the same test files. Ex submitted for Indiana Q1 2023 in QASP: small_tanf_section1(1).txt is reading 0s across the board while accepted with errors (on two cases according to the error report).

@reitermb good callouts, thank you! i've removed the offending margin and added in "Pending" and "No Errors" states to the error reports column. Regarding the inconsistent record/error counts - this is something we're aware of and will be discussing, but having unique CASE_NUMBER values for the records in your test file might help

Sure, the case numbers are 11111111112 and 11111111140. I've also attached the file & error report.

2023-Q1-Active Case Data (14).xlsx
2023.Q1.Active Case Data (2).txt

With those cell fill and table margin fixes committed we should be good on the a11y side!

@reitermb reitermb added Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI and removed Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI labels Oct 2, 2023
@reitermb reitermb self-requested a review October 2, 2023 16:36
Copy link

@reitermb reitermb left a comment

Choose a reason for hiding this comment

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

Confirming that the margin/empty cell fixes are looking good a11y wise! Ready for gov review @ttran-hub @ADPennington
image

@ADPennington ADPennington removed the Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI label Oct 3, 2023
@reitermb reitermb added the Deploy with CircleCI-a11y Deploy to https://tdp-frontend-a11y.app.cloud.gov through CircleCI label Oct 3, 2023
@reitermb
Copy link

reitermb commented Oct 3, 2023

Officially bumping into QASP a11y/dev review post sync with @ttran-hub.

cc @ADPennington

Nothing that blocks QASP/indicates a compliance issue on this ticket but also noting that we'll be doing some follow-on a11y research on this ticket to confirm that the way screen-readers are interacting with the table rowspans is the ideal behavior for those. If not, it might be an area for a follow-on ticket to add more descriptive text to accompany the table via the table's caption attribute or aria properties.

@reitermb reitermb requested a review from ttran-hub October 3, 2023 18:52
@ttran-hub
Copy link
Collaborator

Officially bumping into QASP a11y/dev review post sync with @ttran-hub.

cc @ADPennington

Nothing that blocks QASP/indicates a compliance issue on this ticket but also noting that Thomas, Diana, and I will do some follow-on a11y research on this ticket to confirm that the way screen-readers are interacting with the table rowspans is the ideal behavior for those. If not, it might be an area for a follow-on ticket to add more descriptive text to accompany the table via the table's caption attribute or aria properties.

Thanks @reitermb for the added note on the follow-on a11y research on the way screen readers are interacting with the table rowspans. This should not hold up the PR. a11y 🚀 cc @ADPennington

@ADPennington ADPennington removed the Deploy with CircleCI-a11y Deploy to https://tdp-frontend-a11y.app.cloud.gov through CircleCI label Oct 3, 2023
@reitermb reitermb added the Deploy with CircleCI-a11y Deploy to https://tdp-frontend-a11y.app.cloud.gov through CircleCI label Oct 4, 2023
@ADPennington ADPennington added Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI and removed Deploy with CircleCI-a11y Deploy to https://tdp-frontend-a11y.app.cloud.gov through CircleCI labels Oct 4, 2023
@ADPennington
Copy link
Collaborator

@jtimpe @reitermb @ttran-hub submission history looks great for TANF (note: i referenced other tickets in the screenshot for follow-up issues) but looks a little off for SSP. Please see below.

1612p4
1612p3

@ADPennington ADPennington added Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI and removed Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI labels Oct 4, 2023
Copy link
Collaborator

@ADPennington ADPennington left a comment

Choose a reason for hiding this comment

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

thanks for the quick fix @jtimpe 🚀

1612p5

@ADPennington ADPennington added Ready to Merge and removed raft review This issue is ready for raft review QASP Review Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI labels Oct 4, 2023
@andrew-jameson andrew-jameson merged commit 954edc7 into develop Oct 5, 2023
15 checks passed
@jtimpe jtimpe mentioned this pull request Oct 11, 2023
31 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

As a user, I want to see detailed metadata for each file version.
10 participants