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

feat: add getQuestion instance method to form field schema #103

Merged
merged 11 commits into from
Aug 11, 2020

Conversation

karrui
Copy link
Contributor

@karrui karrui commented Aug 7, 2020

Problem

Whilst migrating utils/question.js, realise that exported function can be inlined into the base field schema instead.

This PR adds a new instance method getQuestion() to retrieve the question for display in form responses.
Tests are also added for the new instance method.

Note that base field schemas still do not have tests written for them. This should be done in a separate PR. Issue #102 is created to track this issue.

Related to #8

Solution

Features:

  • Replace utils/question with FormField.methods.getQuestion to retrieve questions of form fields.
    • utils/question is deleted

Bug fixes:

  • Add validation in table fields to ensure that there must be at least 1 column in the column array.

Tests

  • Submit a form with all fields.
    • The responses sent to the admin/form-filler email for email forms should have the correct questions for all fields (table fields should be ${title} (${column titles separated by comma})
    • The responses sent to the results tab for storage mode forms should also display the correct questions.

@karrui
Copy link
Contributor Author

karrui commented Aug 7, 2020

Tests are failing as the mocked form_fields objects in the tests are not truly a mongoose document :(. Will do a deeper check to see if it is guaranteed to be a form field schema

Tests fixed.

karrui added 2 commits August 7, 2020 14:29
was causing schema building to fail due to expecting a document instead of an object
@karrui karrui force-pushed the refactor/ts-utils-question branch from 1476f87 to 0f31d59 Compare August 7, 2020 06:33
})
})

const createAndReturnFormField = async (formFieldParams, formType) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

the need for this is telling me that we're missing a factory method to create form field objects - we should eventually put one on FieldSchema.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, since FormFields are not registered as a model, we can't create them directly. Wouldn't creating a method still require the model to be registered?

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.

lgtm

@karrui karrui merged commit 2facecf into develop Aug 11, 2020
@karrui karrui deleted the refactor/ts-utils-question branch August 11, 2020 02:54
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.

2 participants