Tech debt on app/assets/javascripts/notes/components/comment_form.vue

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

While fixing this bug: https://gitlab.com/gitlab-org/gitlab-ce/issues/44149 I noticed a few problems that make this notes vue app a little harder to maintain.

Vuex should make our lifes easier, but in this App it's making it a little harder. The bugs we've been fixing have taken a lot more time that they should:

  1. https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17671
  2. https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17043
  3. https://gitlab.com/gitlab-org/gitlab-ce/issues/44149

Reasons found so far:

  1. The state is shared between the store and the view making it hard to track mutations. Loadings and submitting states should be in the store to avoid UI flickering.
  2. Some actions happen outside of the Vue app
  3. Some data mutations from server to the store are hard to understand.

There are a few open issues regarding technical debt in this code, but none have been fixed. In fact we added a little more:

  1. https://gitlab.com/gitlab-org/gitlab-ce/issues/44149
  2. https://gitlab.com/gitlab-org/gitlab-ce/issues/39991
  3. https://gitlab.com/gitlab-org/gitlab-ce/issues/39990
  4. https://gitlab.com/gitlab-org/gitlab-ce/issues/39989

Only 1 was fixed so far: https://gitlab.com/gitlab-org/gitlab-ce/issues/39992

More problems found on app/assets/javascripts/notes/components/comment_form.vue

  1. Things are being tested using jquery: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/javascripts/notes/components/comment_form_spec.js#L91

  2. Some components are in vue_shared folder others are in notes. This was part of the first step of plan to make the code reusable but was never finished or reverted.

  3. A form with a submit button should handle the submit event on the form, not on the button's click event. https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/notes/components/comment_form.vue#L362

I'm concerned that this "little" isolated problems will make our life painful in the future. The 3 bugs listed in here, each took more than a day fix and they shouldn't.

The tree state and mutations are not clear enough.

/cc @timzallmann @fatihacet @iamphill

Edited by 🤖 GitLab Bot 🤖