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: migrate GET /adminform endpoint to Typescript #575

Merged
merged 14 commits into from
Nov 9, 2020
Merged

refactor: migrate GET /adminform endpoint to Typescript #575

merged 14 commits into from
Nov 9, 2020

Conversation

karrui
Copy link
Contributor

@karrui karrui commented Nov 3, 2020

Problem

This PR adds a new admin-form module, and refactors the GET /adminform endpoint to the new layered architecture.

Solution

Features:

  • add FormModel#getDashboardForms static function
  • add FormService#getDashboardForms service function
  • add AdminFormController#handleListDashboardForms handler for GET /adminforms endpoint

Improvements:

  • Use new controller handler in admin-forms.server.routes.js and remove old list function.

Tests

  • add tests for getDashboardForms service function
  • add tests for getDashboardForms model static function

@karrui karrui force-pushed the ref/admin-dashboard-form-ts branch from 58fc92e to 346a988 Compare November 3, 2020 03:15
@karrui karrui mentioned this pull request Nov 5, 2020
Merged
mantariksh
mantariksh approved these changes Nov 9, 2020
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.

minor suggestions!

src/app/modules/form/admin-form/admin-form.controller.ts Show resolved Hide resolved
tests/unit/backend/models/form.server.model.spec.ts Show resolved Hide resolved
tests/unit/backend/models/form.server.model.spec.ts Show resolved Hide resolved
tests/unit/backend/models/form.server.model.spec.ts Outdated Show resolved Hide resolved
src/app/models/form.server.model.ts
.where('status')
.ne(Status.Archived)
// Project selected fields.
.select('_id title admin lastModified status form_fields')
Copy link
Contributor

Choose a reason for hiding this comment

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

might be cleaner to extract this into a constants file somewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only used here, no need to extract for now

@karrui karrui mentioned this pull request Nov 9, 2020
Merged
@karrui karrui merged commit 2dbf0bd into develop Nov 9, 2020
@karrui karrui deleted the ref/admin-dashboard-form-ts branch November 9, 2020 08:00
@tshuli tshuli mentioned this pull request Nov 10, 2020
Merged
@tshuli tshuli mentioned this pull request Aug 25, 2023
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

@liangyuanruo liangyuanruo liangyuanruo approved these changes

@tshuli tshuli Awaiting requested review from tshuli

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

Successfully merging this pull request may close these issues.
3 participants
@karrui @liangyuanruo @mantariksh