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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

More survey improvements #3133

Merged
merged 22 commits into from
Apr 6, 2018
Merged

More survey improvements #3133

merged 22 commits into from
Apr 6, 2018

Conversation

deivid-rodriguez
Copy link
Contributor

馃帺 What? Why?

This PR sits on top of #3091. It keeps refactoring surveys (mainly the frontend), and adds a couple of noticiable changes:

  • Question type selector is now correctly disabled once the survey is answered by someone.
  • Client side validation errors are now displayed when filling the survey.

馃搶 Related Issues

None.

馃搵 Subtasks

  • Add CHANGELOG entry

馃摲 Screenshots (optional)

None.

@mrcasals
Copy link
Contributor

mrcasals commented Apr 5, 2018

@deivid-rodriguez after merging #3091 I'm getting conflicts 馃槥

Since it might be different depending on the type of field. For example,
for a text field dependent on a checkbox, we'll need to check the
"checked" property of the checkbox, not its value.
Keeping them on a single column is weird and very unflexible.
Answer is a different model not involved here, so this version is less
confusing, I think.
This is a similar refactoring to what was done in the backend. Instead
of using isolated form helpers, use a proper form builder. This came
with the fix/feature of adding proper support for client side
validations, improving user experience and reducing server load.
@deivid-rodriguez
Copy link
Contributor Author

Fixed, sorry about that, I'm not sure why all those conflicts are generated on my "PRs on top of PRs"... 馃槥

@deivid-rodriguez
Copy link
Contributor Author

Data migrations are tough, I found a few problems in the migration introduced in this PR. Keeping the WIP label until I get to fix them!

@mrcasals
Copy link
Contributor

mrcasals commented Apr 5, 2018

Fixed, sorry about that, I'm not sure why all those conflicts are generated on my "PRs on top of PRs"... 馃槥

@deivid-rodriguez maybe it's due to squashing the commits on merge? 馃槙

@deivid-rodriguez
Copy link
Contributor Author

I hadn't think about that, and it would make sense indeed...

PostgreSQL casting was saving the new text bodies as "\"value\"".
@deivid-rodriguez
Copy link
Contributor Author

Fixed the migration and a few more issues regarding field disabling. Should be ready now.

@josepjaume josepjaume merged commit b70aef9 into master Apr 6, 2018
@josepjaume josepjaume deleted the more_survey_improvements branch April 6, 2018 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants