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

refactor: Convert form-feedback.client.factory.js, CsvGenerator.js to TypeScript #1899

Merged
merged 23 commits into from
May 27, 2021
Merged

refactor: Convert form-feedback.client.factory.js, CsvGenerator.js to TypeScript #1899

merged 23 commits into from
May 27, 2021

Conversation

tshuli
Copy link
Contributor

@tshuli tshuli commented May 17, 2021 •
edited
Loading

Problem

Closes #1509

Tests

  • Create email mode form. Check that feedback tab shows no feedback received.
  • Check that form feedback can be submitted successfully. Do this several times.
  • Check that submitted feedback and count appears correctly on feedback tab.
  • Check that submitted feedback can be downloaded.
  • Repeat above with storage mode form.

@tshuli tshuli marked this pull request as ready for review May 17, 2021 06:38
karrui
karrui requested changes May 18, 2021
View reviewed changes
src/app/modules/form/public-form/__tests__/public-form.controller.spec.ts Outdated Show resolved Hide resolved
src/types/form_feedback.ts Outdated Show resolved Hide resolved
src/app/modules/form/public-form/public-form.controller.ts Outdated Show resolved Hide resolved
src/public/modules/forms/admin/directives/view-feedback.client.directive.js Outdated Show resolved Hide resolved
src/public/modules/forms/admin/directives/view-feedback.client.directive.js Outdated Show resolved Hide resolved
src/public/modules/forms/base/components/feedback-form.client.component.js Outdated Show resolved Hide resolved
src/public/modules/forms/helpers/CsvGenerator.ts Outdated Show resolved Hide resolved
src/public/modules/forms/services/FormFeedbackService.ts Outdated Show resolved Hide resolved
src/public/modules/forms/services/__tests__/FormFeedbackService.spec.ts Outdated Show resolved Hide resolved
src/public/modules/forms/services/__tests__/FormFeedbackService.spec.ts Outdated Show resolved Hide resolved
@karrui
Copy link
Contributor

karrui commented May 18, 2021

Oh, also add tests for the base CsvGenerator class?

mantariksh
mantariksh suggested changes May 18, 2021
View reviewed changes
src/types/form_feedback.ts Outdated Show resolved Hide resolved
src/public/modules/forms/base/components/feedback-form.client.component.js Outdated Show resolved Hide resolved
src/public/modules/forms/helpers/CsvGenerator.ts Outdated Show resolved Hide resolved
src/public/modules/forms/helpers/CsvGenerator.ts Outdated Show resolved Hide resolved
src/app/modules/form/public-form/public-form.controller.ts Outdated Show resolved Hide resolved
src/public/modules/forms/services/FormFeedbackService.ts Outdated Show resolved Hide resolved
src/public/modules/forms/services/FormFeedbackService.ts Outdated Show resolved Hide resolved
src/public/modules/forms/services/FormFeedbackService.ts Outdated Show resolved Hide resolved
src/public/modules/forms/admin/directives/view-feedback.client.directive.js Outdated Show resolved Hide resolved
src/public/modules/forms/admin/directives/view-feedback.client.directive.js Outdated Show resolved Hide resolved
@tshuli tshuli requested review from mantariksh and karrui May 24, 2021 03:50
karrui
karrui approved these changes May 24, 2021
View reviewed changes
Copy link
Contributor

@karrui karrui left a comment

Choose a reason for hiding this comment

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

lgtm with nits

src/public/modules/forms/helpers/__tests__/CsvGenerator.spec.ts Outdated Show resolved Hide resolved
src/public/modules/forms/helpers/__tests__/CsvGenerator.spec.ts Outdated Show resolved Hide resolved
src/public/services/FormFeedbackService.ts Outdated Show resolved Hide resolved
src/types/api/form-feedback.ts Outdated Show resolved Hide resolved
mantariksh
mantariksh reviewed May 24, 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.

looks good other than fixing the type on the controller. thanks for writing such detailed tests!

src/app/modules/form/public-form/public-form.controller.ts Outdated Show resolved Hide resolved
src/public/modules/forms/admin/directives/view-feedback.client.directive.js Outdated Show resolved Hide resolved
tshuli added 20 commits May 25, 2021 21:08
@tshuli
refactor: shift feedback types to src/types
cc259da
@tshuli
chore: define FormFeedbackDto type
ab91f58
@tshuli
chore: submitFormFeedback in controller to return submitted form feed…
e6d99b3 
…back object
@tshuli
refactor: convert CsvGenerator to typescript
3670343
@tshuli
refactor: migrate form-feedback.client.factory to FormFeedbackService
7d20e45
@tshuli
chore: add tests for FormFeedbackService
659a2bd
@tshuli
chore: revert controller to return submitted form feedback object
c919afa
@tshuli
chore: shift dto types to /types/api
1b3ed22
@tshuli
chore: use $q.when
3856e23
@tshuli
chore: remove typings from JSDocs
5d51c0f
@tshuli
chore: shift FeedbackCsvGenerator to separate file
1f6a8db
@tshuli
chore: add tests for FeedbackCsvGenerator
acd3ac6
@tshuli
chore: add tests for CsvGenerator
e646c9f
@tshuli
chore: shift FormFeedbackService to src/public/services
f9eb920
@tshuli
chore: remove unnecessary return
ce15b86
@tshuli
chore: remove blank lines
581f953
@tshuli
refactor: assign params for constructor to constant
eda5aef
@tshuli
chore: replace with generic error message
7e9c3e5
@tshuli
chore: rename form_feedback and encrypt_submission to be underscored
8444c94
@tshuli
chore: fix response type on submitFormFeedback
d7a7fcd
@tshuli
Copy link
Contributor Author

tshuli commented May 25, 2021

thanks @karrui and @mantariksh for reviewing

@mantariksh need an approval from you!

@mantariksh
Copy link
Contributor

Closes #1837 also

@mantariksh mantariksh merged commit 42fc11a into develop May 27, 2021
@mantariksh mantariksh mentioned this pull request May 27, 2021
Closed
@liangyuanruo liangyuanruo deleted the ref/feedback-factory branch May 31, 2021 12:51
@liangyuanruo liangyuanruo mentioned this pull request May 31, 2021
Closed
@yong-jie yong-jie mentioned this pull request Jun 1, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mantariksh mantariksh mantariksh approved these changes

@karrui karrui karrui 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.

Convert form-feedback.client.factory.js, CsvGenerator.js to TypeScript
3 participants
@tshuli @karrui @mantariksh