Skip to content

Conversation

NehaBroad
Copy link
Contributor

@NehaBroad NehaBroad commented Mar 24, 2022

Update Sign Code of Conduct on DAR page
Screen Shot 2022-03-24 at 12 51 38 PM

Email:

  • Update Sign of Conduct case
  • Remove flex from bullet

image


PR checklist

  • This PR meets the Acceptance Criteria in the JIRA story
  • The JIRA story has been moved to Dev Review
  • This PR includes appropriate unit tests
  • I have run and tested this change locally
  • I have run the E2E tests on ths change against my local UI and/or API server with yarn test-local or yarn test-local-devup
  • If this includes a UI change, I have taken screen recordings or screenshots of the new behavior and notified the PO and UX designer
  • If this includes an API change, I have updated the appropriate Swagger definitions and notified API consumers
  • If this includes a new feature flag, I have created and linked new JIRA tickets to (a) turn on the feature flag and (b) remove it later


@Test
public void resendWelcomeEmail_messagingException() throws MessagingException {
config.access.tiersVisibleToUsers =
Copy link
Collaborator

Choose a reason for hiding this comment

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

please add a comment explaining why this is necessary

Copy link
Collaborator

Choose a reason for hiding this comment

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

please also restore the deprecated versions of these tests

Copy link
Collaborator

Choose a reason for hiding this comment

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

This functionality will no longer be used as of the next production release, so we decided that the tests do not provide enough utility for the time needed to bring them back.

}

@Test
public void sendUserInstructions_none_deprecated() throws MessagingException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

these new tests are not correct. We already have the "deprecated" versions. We need the undeprecated versions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

also need a "deleted instructions" undeprecated test

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added a test to handle the un-deprecated case.

}

@Test
public void testSendWelcomeEmail_invalidEmail_RTAndCt() throws MessagingException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add local vars to make it clear what these 6 tests are doing.

Like: boolean userHasRt = true

Copy link
Collaborator

Choose a reason for hiding this comment

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

Decided to not pursue this at this time. If we would like to pursue this convention in the future, we can discuss it.

<img src="${BULLET_1}">
</div>
</td>
<td colspan="3">
Copy link
Contributor

Choose a reason for hiding this comment

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

Bullet and text seems little too far apart for me. I'm curios about colspan="3", not colspan="2".

Copy link
Collaborator

Choose a reason for hiding this comment

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

I updated the spacing and put a screenshot in the description. What do you think?

@jmthibault79 jmthibault79 force-pushed the nsaxena/820WelcomeEmailTry branch from 8e8bf4f to a73f372 Compare March 30, 2022 20:59
@jmthibault79 jmthibault79 force-pushed the nsaxena/820WelcomeEmailTry branch from a73f372 to d05254a Compare March 30, 2022 21:15
Copy link
Contributor

@aweng98 aweng98 left a comment

Choose a reason for hiding this comment

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

Reviewed in mob! Looks Good!

@evrii evrii merged commit 28974a8 into main Apr 5, 2022
@evrii evrii deleted the nsaxena/820WelcomeEmailTry branch April 5, 2022 16:21
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.

5 participants