-
Notifications
You must be signed in to change notification settings - Fork 3
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
1755 - added ssp boolean to stt model #2211
Conversation
tdrs-backend/tdpservice/stts/management/commands/populate_stts.py
Outdated
Show resolved
Hide resolved
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #2211 +/- ##
===========================================
- Coverage 93.86% 93.80% -0.07%
===========================================
Files 88 88
Lines 2396 2405 +9
Branches 318 320 +2
===========================================
+ Hits 2249 2256 +7
- Misses 110 112 +2
Partials 37 37
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@reitermb For A11y I am interested to know if anything needs to happen when an OFA Admin selects and STT with SSP access. |
tdrs-backend/tdpservice/stts/migrations/0006_alter_stt_filenames.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from an a11y standpoint. Radio buttons behave as radio buttons should, i.e:
- Label is read for the group
- Navigation/selection within the group happens via arrow keys rather than tabbing
No curveballs here so it should be good to go based on conversation with @ttran-hub yesterday.
Video showing behavior:
https://user-images.githubusercontent.com/28515874/199581520-a2f2349e-19a9-4e27-bc7e-28c5530a503c.mp4
Just one question re: ticket scope, I noticed that we're not changing the heading2 ( search
result) heading based off the TANF / SSP selection. Was there a decision to make that follow-on work? (This is AC #5 in the ticket)
@n0remac Miles is referring to this AC: |
f0b5096
to
96ed2bb
Compare
d5b909c
to
7d19af5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good. Migrations works locally, the TANF
vs SSP-MOE
radio button works great for submitting files. Just a couple notes.
- There's an
IntegrityError
thrown inside thesftp_task
because thedata_file.filename()
currently returnsnull
for SSP submissions. However, I think this will be fixed by As a user, I need the filename for SSP section to be correctly assigned after uploading the file #2154.
- The radio button works great for uploading data files, but I'm not sure it's used to
GET
the search results again. I don't know if it qualifies as scope creep on the ticket, but I'm not sure the search functionality is consistent anymore. @ADPennington @andrew-jameson
- Lastly, Fix/992 notify data file search selection changes #2209 changes how the frontend form works and I think it'd be useful to either update the implementation of the radio button when it merges or create a follow-on ticket to do so.
Please see #2236 for latest |
Summary of Changes
Provide a brief summary of changes
Pull request closes #1755
I have included code to allow the SSP to be modified in the DAC for ease of access in testing.
Demo on Raft:
dev_ssp_file_type_1.mov
dev_ssp_file_type_2.mov
Older local demo:
Added SSP radio buttons to frontend when SSP is added for STT. Passes value to backend through DRF on submit.
Screen.Recording.2022-10-28.at.3.32.50.PM.mov
How to Test
As a Non OFA Admin belonging to an STT with SSP set to
yes
go to the datafiles page and theFile Type
section will be visible.As an OFA Admin the
File Type
section will become visible when an STT with SSP set toyes
is selected.Deliverables
More details on how deliverables herein are assessed included here.
Deliverable 1: Accepted Features
Checklist of ACs:
Deliverable 2: Tested Code
CodeCov Report
comment in PR)CodeCov Report
comment in PR)Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
iamjolly
andttran-hub
using Accessibility Insights reveal any errors introduced in this PR?Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure
Deliverable 8: User Research
Research product(s) clearly articulate(s):