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

Form fixes #2000

Merged
merged 11 commits into from Jan 29, 2019

Conversation

Projects
None yet
2 participants
@noirbizarre
Copy link
Member

noirbizarre commented Jan 28, 2019

This PR fixes some major bugs in forms:

  • forms errors were mispositionned and not injected into vue.js forms
  • server side error were silently ignored
  • success notification was always dispatched even on on transport or server-side error
  • some forms were using 2 differents models (Dataset and Resource) for the same thing
  • some forms were never able to dispatch a success notification
  • hide-notification was never true
  • there was no deletion success notification

The fixes are:

  • rely on the updated event to consider a form successfuly submitted and only then route to the next view or close modal and show the success notification
  • ensure jQuery.validate properly inject errors into vue.js forms
  • ensure server-side errors are properly injected into vue.js forms
  • ensure only one object/model is managed by each form
  • use the same notification logic for deletion
  • dispatch a notification for non-field errors
  • fix the error labels style

This PR is a quick and temporary fix but this need a major refactoring (Vuex, promise, async... whatever)

@noirbizarre noirbizarre added this to the 1.6.3 milestone Jan 28, 2019

@noirbizarre noirbizarre requested a review from opendatateam/etalab Jan 28, 2019

@noirbizarre noirbizarre force-pushed the noirbizarre:form-fixes branch from 6e16e30 to 4ddda3c Jan 28, 2019

@abulte

abulte approved these changes Jan 28, 2019

Copy link
Member

abulte left a comment

This looks way cleaner and more robust 💪

Show resolved Hide resolved js/utils.js Outdated
@noirbizarre

This comment has been minimized.

Copy link
Member Author

noirbizarre commented Jan 28, 2019

Yep, I tried to be stricter on separation of concerns between forms, views and models.
But, OMG, I'd like to have Vuex and async !!!!

@noirbizarre noirbizarre force-pushed the noirbizarre:form-fixes branch 3 times, most recently from d06a17b to 4a4b142 Jan 28, 2019

@noirbizarre noirbizarre changed the title WIP: Form fixes Form fixes Jan 29, 2019

@noirbizarre noirbizarre force-pushed the noirbizarre:form-fixes branch from d0edfda to 37a3dc9 Jan 29, 2019

@noirbizarre noirbizarre merged commit fe22588 into opendatateam:master Jan 29, 2019

3 checks passed

ci/circleci: assets Your tests passed on CircleCI!
Details
ci/circleci: dist Your tests passed on CircleCI!
Details
ci/circleci: python Your tests passed on CircleCI!
Details

@noirbizarre noirbizarre deleted the noirbizarre:form-fixes branch Jan 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.