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

refactor: migrate preview admin form endpoint to Typescript #864

Merged
merged 20 commits into from
Feb 16, 2021

Conversation

karrui
Copy link
Contributor

@karrui karrui commented Dec 14, 2020

Problem

Migrates :formId/adminform/preview endpoint to Typescript.

The choice of public fields to display is now in each database model's domain; see:

  • UserModel.getPublicView
  • AgencyModel.getPublicView
  • FormModel.getPublicView

Solution

Features:

  • feat(AdminFormCtl): add (and use) handlePreviewAdminForm handler fn
  • feat(Form): replace util fn with form model method getPublicView

Tests

Controller tests have been added.
add tests for FormModel#getPublicView instance method
add tests for FormSvc#getFormPublicView

@karrui karrui requested a review from mantariksh December 14, 2020 04:08
@karrui karrui requested a review from liangyuanruo December 17, 2020 10:07
@karrui
Copy link
Contributor Author

karrui commented Dec 17, 2020

Rerequesting review since the extraction of form instance method was slightly more complex than it seemed (and also upped the robustness of the public form view; now it ensures the form is populated before returning the public view)

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 other than yr's comments!

Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

Some comments for your consideration, please.

@karrui karrui requested a review from liangyuanruo December 21, 2020 06:23
@karrui
Copy link
Contributor Author

karrui commented Dec 21, 2020

Ready for rereview

@liangyuanruo
Copy link
Contributor

Closing this PR for the time being until @karrui 's involvement on other project is reduced.

@karrui karrui reopened this Feb 15, 2021
# Conflicts:
#	src/app/modules/form/__tests__/form.utils.spec.ts
#	src/app/modules/form/form.utils.ts
#	src/app/modules/submission/email-submission/email-submission.service.ts
@karrui karrui requested a review from mantariksh February 15, 2021 05:26
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

Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

I notice the consistent use of getPublicView() which is great, so let's create a new interface in order to embed that consistency:

interface PublicView<T> {
  getPublicView(): T
}

interface IAgencySchema extends IAgency, Document, PublicView<PublicAgency> {...}

@karrui karrui merged commit d332fc1 into develop Feb 16, 2021
@karrui karrui deleted the ref/preview-admin-form branch February 16, 2021 04:34
@karrui karrui mentioned this pull request Feb 23, 2021
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