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.

Sign up for GitHub

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

Jump to bottom

feat: add response ID to thank you page #1855

Merged
merged 15 commits into from
May 19, 2021
Merged

feat: add response ID to thank you page #1855

merged 15 commits into from
May 19, 2021

Conversation

chowyiyin
Copy link
Contributor

@chowyiyin chowyiyin commented May 11, 2021 •
edited
Loading

Problem

This PR adds the response ID to the Thank You page. For details, see #1409.

Closes #1409

Screenshots

Screenshot 2021-05-12 at 7 12 13 PM

Screenshot 2021-05-17 at 10 00 37 AM

Screenshot 2021-05-19 at 12 21 06 PM

Manual Tests

  • Create email field and turn on email confirmation. Check that dummy response ID is present in email confirmation preview in edit field modal
  • Create email field in email mode and submit form
  • Check that correct response ID (submission ID, not form ID) is displayed above time of submission
  • Check that response ID is present in email confirmation
  • Enter preview mode and repeat above checks
  • Create email field in storage mode and submit form
  • Check that correct response ID (submission ID, not form ID) is displayed above time of submission
  • Check that response ID is present in email confirmation
  • Enter preview mode and repeat above checks

@chowyiyin chowyiyin changed the title Add response ID to thank you page build: add response ID to thank you page May 11, 2021
seaerchin
seaerchin reviewed May 12, 2021
View reviewed changes
Copy link
Contributor

@seaerchin seaerchin left a comment

Choose a reason for hiding this comment

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

looks good! but have some minor comments about small stuff

src/public/modules/forms/base/css/end-page.css Outdated Show resolved Hide resolved
src/public/modules/forms/base/directives/submit-form.directive.js Outdated Show resolved Hide resolved
@seaerchin
Copy link
Contributor

seaerchin commented May 12, 2021 •
edited
Loading

btw i think cos you branched off another PR, you'd rebase off develop (once it's merged) so that old commits aren't seen or if it isn't, set the compared branch as the base branch!

@chowyiyin chowyiyin changed the base branch from develop to colorthemes May 12, 2021 06:25
@chowyiyin chowyiyin changed the base branch from colorthemes to develop May 12, 2021 08:20
@karrui
Copy link
Contributor

karrui commented May 12, 2021

can i request for some screenshots too since this is a client side change?

@chowyiyin
refactor: changed response key from id to submissionId and removed pa…
9c365d5 
…ramters to improve readability and handled error for response parsing
mantariksh
mantariksh suggested changes May 18, 2021
View reviewed changes
Copy link
Contributor

@mantariksh mantariksh left a comment

Choose a reason for hiding this comment

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

is it possible to increase the padding between the reference number, the horizontal line and the body of the actual email confirmation?

src/app/services/mail/mail.types.ts Outdated Show resolved Hide resolved
src/app/services/mail/mail.service.ts Outdated Show resolved Hide resolved
src/public/modules/forms/services/submissions.client.factory.js Show resolved Hide resolved
src/public/modules/forms/services/submissions.client.factory.js Show resolved Hide resolved
src/public/modules/forms/services/submissions.client.factory.js Outdated Show resolved Hide resolved
src/public/modules/forms/base/directives/submit-form.directive.js Outdated Show resolved Hide resolved
mantariksh
mantariksh reviewed May 19, 2021
View reviewed changes
Copy link
Contributor

@mantariksh mantariksh left a comment

Choose a reason for hiding this comment

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

lgtm, to test on staging before approval

src/app/services/mail/mail.types.ts Outdated Show resolved Hide resolved
@mantariksh
Copy link
Contributor

also could you add more manual tests to cover all the cases? eg

  • storage mode
  • email confirmation preview in edit field modal
  • actual email confirmation
  • submission in admin preview mode for both email and storage mode
  • email confirmation in admin preview mode for both email and storage mode

@chowyiyin chowyiyin changed the title build: add response ID to thank you page feat: add response ID to thank you page May 19, 2021
@chowyiyin chowyiyin merged commit 035da37 into develop May 19, 2021
@mantariksh mantariksh mentioned this pull request May 25, 2021
Merged
@karrui karrui deleted the responseID branch August 17, 2021 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seaerchin seaerchin seaerchin left review comments

@mantariksh mantariksh mantariksh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add response ID to thank you page and email confirmation
4 participants
@chowyiyin @seaerchin @karrui @mantariksh