Skip to content

Refactor issuables list to Vue, part 1

Martin Hanzel requested to merge mh/vue-issuables-list-ee into master

What does this MR do?

Re-work of !14100 (closed). Refactors the issuable list component into Vue.

What's done

  • Rendering issuables on the group issues page
  • Toggling bulk edit by interfacing with the bulk edit sidebar.
  • Performing the bulk edit
  • Reordering issues
  • Pagination
  • Filtering
  • Sorting
  • Empty states
  • Tests: !15689 (merged)

What's still pending

  • Scoped labels badge and popover. We need a new component for scoped label.
  • User popover deserves its own component. Right now, it's attaching events to Vue-rendered elements.
  • The issues/MR API needs to return the reference path of the issuable: gitlab-org/gitlab-ce#66655

Differences from !14100 (closed)

  • Enabled this feature behind the vue_issuables_list feature flag.
  • Removed the VueX store. This component does not have a complex hierarchy and data is more than sufficient for reactivity. This saved around 200 LoC and several files.
  • Simplified the layout and removed several CSS classes, preferring Bootstrap utilities instead. This saved about 100 LoC.
  • Added ~/lib/utils/pubsub.js, which is a simple async pub/sub mechanism. This enables the Vue component to talk to the bulk edit sidebar, which is a jQuery component, without needing a separate mediator.
    • !14100 (closed) did something very similar, but used the VueX store as a message store.
    • The existence of this file, and the best way to make Vue components communicate with jQuery, is being discussed in gitlab-org/gitlab-ce!31868
  • The Vue component's HTML is isolated - no external component mutates the DOM anymore (looking at you, Bulk Edit Sidebar!)

Next steps

  • app/assets/javascripts/issuable_bulk_update_actions.js and app/assets/javascripts/issuable_bulk_update_sidebar.js contain jQuery code. There's a strong coupling to the issuable's DOM, as the bulk edit sidebar and its dropdowns find out which issues are selected, as well as selected issues' properties like labels, from inspecting the DOM. This should be refactored to a state store or pub/sub mechanism so that the input data are made explicit and so that it doesn't break if we change the views.

For maintainers: how to merge

This is part 1 of a pair of MRs. Part 2 is here: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/15689

Part 2 contains tests and code changes to satisfy tests. Part 1 contains business logic.

Neither part has EE-specific code, and can be cleanly backported to CE.

TO MERGE:

  1. Merge part 2 into mh/vue-issuables-list-ee. Part 2 is based on that branch, so the pipelines on Part 1 should then pass.
  2. Merge Part 1 into master.
  3. 🐝 happy.

Dependencies

/cc @donaldcook

To see for yourself, enable the :vue_issuables_list feature flag and go to any group issues page, like http://localhost:3001/groups/h5bp/-/issues

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Coung Ngo

Merge request reports