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

[EASI-3098] Share Model Plan #665

Merged
merged 22 commits into from
Sep 1, 2023

Conversation

OddTomBrooks
Copy link
Contributor

@OddTomBrooks OddTomBrooks commented Aug 16, 2023

EASI-3098

Changes and Description

  • Share Model email template
  • View Filter enum

FE Changes

  • Added UI for Share on modal
  • Added clipboard copy for entire model plan as well as filtered view link
  • Mutation to send model plan
  • Success/error states on tasklist/readonly
  • Updated unit tests
Screenshot 2023-08-29 at 10 49 59 AM Screenshot 2023-08-29 at 10 50 27 AM Screenshot 2023-08-29 at 10 50 59 AM Screenshot 2023-08-29 at 11 03 40 AM

How to test this change

*From both task list and readonly

  • Verify if opening modal from filtered view, that is gets autopopulated
  • Verify button is disabled if not filter view is selected
  • Verify mutation passes successfully if valid email - success Alert component rendered
  • Verify mutation fail on inproper email address - error Alert component rendered
  • Verify copy link to clipboard is successful on both all plan and filter view selections

PR Author Review Checklist

  • Met the ticket's acceptance criteria, or will meet them in a subsequent PR.
  • Added or updated tests for backend resolvers or other functions as needed.
  • Added or updated client tests for new components, parent components, functions, or e2e tests as necessary.
  • Updated the Postman Collection if necessary.

PR Reviewer Guidelines

  • It's best to pull the branch locally and test it, rather than just looking at the code online!
  • Check that all code is adequately covered by tests - if it isn't feel free to suggest the addition of tests.
  • Always make comments, even if it's minor, or if you don't understand why something was done.

@OddTomBrooks OddTomBrooks marked this pull request as ready for review August 21, 2023 19:26
@OddTomBrooks OddTomBrooks requested review from a team as code owners August 21, 2023 19:26
@OddTomBrooks OddTomBrooks requested review from ClayBenson94 and removed request for a team August 21, 2023 19:26
Copy link
Collaborator

@ClayBenson94 ClayBenson94 left a comment

Choose a reason for hiding this comment

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

Great job on these changes! 🎉

Just noting some changes we'll want to pull in regarding multiple model categories, and some smaller feedback around error handling that isn't as big of a priority

pkg/graph/resolvers/model_plan.go Outdated Show resolved Hide resolved
pkg/graph/resolvers/model_plan.go Outdated Show resolved Hide resolved
pkg/graph/resolvers/model_plan.go Show resolved Hide resolved
@ClayBenson94
Copy link
Collaborator

We might also want to handle the case of an empty string as the message in the backend. Potentially a zero.String could be useful for that check!

image

@OddTomBrooks OddTomBrooks requested a review from a team as a code owner August 22, 2023 19:42
@OddTomBrooks
Copy link
Contributor Author

We might also want to handle the case of an empty string as the message in the backend. Potentially a zero.String could be useful for that check!

image

In this case I was able to resolve it with a simple template modification

@StevenWadeOddball StevenWadeOddball removed the request for review from a team August 22, 2023 19:52
@patrickseguraoddball patrickseguraoddball marked this pull request as draft August 29, 2023 15:22
@patrickseguraoddball patrickseguraoddball marked this pull request as ready for review August 29, 2023 18:51
Copy link
Contributor

@garyjzhao garyjzhao left a comment

Choose a reason for hiding this comment

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

FE is working as described

@ClayBenson94 ClayBenson94 merged commit 78e5deb into main Sep 1, 2023
9 of 10 checks passed
@ClayBenson94 ClayBenson94 deleted the feature/EASI-3098_share_model_plan_backend branch September 1, 2023 12: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.

4 participants