Skip to content

Conversation

Forfold
Copy link
Contributor

@Forfold Forfold commented Jan 6, 2022

This PR adds cypress tests for the admin outgoing message logs PR (#2061). I have the base of this PR to merge into that branch: message-logs-frontend, though we can change it to master if that gets merged in first.

Out of scope:

  • Load more test w/ support function to create many logs at once
  • Asserting destination (phone numbers are given with a different format from DB, notification channels are evaluated differently)
  • Provider ID/Source info
  • It would be nice to have a one-to-one API to create a DebugMessage and return a DebugMessage. Would need to do work around message type + status and picking the correct strings if one is not provided in options. e.g. if type: 'failed' is given, find a good way return c.pickOne([possible failed status message strings]) for the status.

@Forfold Forfold marked this pull request as ready for review January 6, 2022 21:28
@Forfold Forfold changed the title Message logs cypress ui/cypress: admin message logs Jan 6, 2022
@arurao arurao self-requested a review January 10, 2022 18:26
@Forfold Forfold requested a review from pnengchu January 12, 2022 17:27
Copy link
Contributor

@arurao arurao left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 15 to 18
# @date-io/luxon@1 requires luxon@1
- dependency-name: 'luxon'
# luxon@1 requires @types/luxon@1
- dependency-name: '@types/luxon'
Copy link
Member

Choose a reason for hiding this comment

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

Merge issue? I see there are Makefile changes too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I thought I fixed it with the revert. Yeah

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure why it's all showing up since I merged message-logs-frontend into this branch. I reverted the accidental master merge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed now. I was trying to get the changes from #2088 into this so it could pass on CI

@Forfold Forfold merged commit 81b189d into message-logs-frontend Jan 14, 2022
@Forfold Forfold deleted the message-logs-cypress branch January 14, 2022 17:25
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