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

[DUOS-2025][risk=no] Remove IT / SO director fields from applicant information section #1848

Merged
merged 3 commits into from
Oct 26, 2022

Conversation

connorlbark
Copy link
Contributor

Addresses

https://broadworkbench.atlassian.net/browse/DUOS-2025


Have you read Terra's Contributing Guide lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

@connorlbark connorlbark marked this pull request as ready for review October 17, 2022 17:24
@connorlbark connorlbark requested a review from a team as a code owner October 17, 2022 17:24
Copy link
Contributor

@quazi-h quazi-h left a comment

Choose a reason for hiding this comment

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

Changes look good so far, running locally, the SO and IT Director have a single field each that has the name and email for the individuals. One nit I have is that it might be clearer to have those fields labeled as "Signing Official" and "IT Director" without the "Email" in the header since the value of those fields contains both name and (if available) email, but otherwise looks good and we don't have any duplicate data showing up under the Applicant Information slab.

@JVThomas
Copy link
Contributor

Changes look good so far, running locally, the SO and IT Director have a single field each that has the name and email for the individuals. One nit I have is that it might be clearer to have those fields labeled as "Signing Official" and "IT Director" without the "Email" in the header since the value of those fields contains both name and (if available) email, but otherwise looks good and we don't have any duplicate data showing up under the Applicant Information slab.

Agreed. It was included as a note in the ticket itself

Note: Remove the email columns for SO and IT Director. These values are essentially the same value, just displayed in two places. 

@connorlbark
Copy link
Contributor Author

ohhh good catch! just updated the labels @quazi-broad @JVThomas

Copy link
Contributor

@JVThomas JVThomas left a comment

Choose a reason for hiding this comment

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

Code looks good to me (tested locally)

@connorlbark connorlbark merged commit d097c97 into develop Oct 26, 2022
@connorlbark connorlbark deleted the DUOS-2025 branch October 26, 2022 18:05
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