Skip to content
Snippets Groups Projects

Issue discussions Vue refactor

Merged Fatih Acet requested to merge issue-discussions-refactor into master

This MR is about to refactor Issue discussions with Vue to allow us to build new features onto it, like starting a new thread from a non-discussion comment.

Related with #30299 (closed) and !11687 (closed)

/cc @DouweM

List of things to do as discussed with @jschatz1 in this MR

  • You can't stop the polling... For when you change tabs. Can't use the current polling function because VueX.
  • When you poll you lose the emojis.
  • [ ] After a Slash command the state is not updating. (This also doesn't happen on master or gitlab.com, waiting on @DouweM feedback on this) issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/35929
  • Using a Vue Instance to pass events to jQuery code. Remove the Vue instance to stop providing events.
  • If you close the issue in issue buttons. If you do something in the sidebar. Nothing updated.
  • When you leave the page the reply should still be there.
  • Make sure we show first class action errors properly.

List of bugs @iamphill found:

  • Alignment seems all wrong
  • JS error when saving edit on note:
  • Can't reply on a discussion
  • [ ] I wonder if we should trigger a poll when we close/re-open the issue or apply slash commands issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/35929
  • I'm not sure how I did it, but I managed to get the issue state as closed but the Close issue button was still there: issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/35929
  • Toggle discussion doesn't work
  • Added a award emoji to a single note got an error but the emoji stayed there even after a refresh
  • Register/Sign-in link lead to nowhere
  • Submit button doesn't disable when submitting a comment
  • Linking to a comment doesn't work
  • Slash commands should be disabled on edit comments
  • It is possible to submit an empty reply to a discussion

List of things @jschatz1 said to not fix in this MR:

  • We are updating using the window.gl obj and updating that obj through vue.
  • No tests for the VueX part.
  • No tests for the Vue components
### Frontend checklist
  • Render single note
  • Render discussion
    • Implement toggle control for discussions
    • Show register and sign in links in discussion footer for anon user
  • Render system notes
  • First class note actions
    • Handle note edit
    • Handle note delete
    • Handle discussion reply
    • Handle new comment created
    • Handle new discussion created
  • First class issue actions
    • Handle issue close
    • Handle comment and close
    • Handle start discussion and close
    • Handle issue reopen
    • Handle comment and reopen
    • Handle start discussion and reopen
    • Make sure that we updating the issue badge after closing/reopening the issue
  • Award Emoji
    • Create award emoji list
    • Handle adding new emoji from emoji dropdown
    • Handle adding new emoji from emoji list
    • Handle emoji click in emoji list
    • Highlight already awarded emoji
    • Handle mutual exclusive emojis
    • Render :thumbsup: and :thumbsdown: at the beginning of the list
    • Disable adding :thumbsup: and :thumbsdown: to your own comment
  • Realtime
    • Implement polling for changes
    • Handle response and render new notes/discussions
    • Refactor polling mechanism to use next generation polling
  • Known issues, improvements, things to do later
    • Fix syntax highlighting
    • Fix KaTex rendering
    • Fix to do list editability
    • Make sure that we create dummy note placeholders while requesting
    • Make sure that we implement Slash commands and autocompletes properly
    • Make sure that up arrow puts last comment into edit mode
    • Create empty comment widget with register and sign in links if user has no permission
    • Remove award emoji related vendored svg's and use them from icons folder
    • Timestamp needs to link to #note_<id>
    • When anchor is #note_<id>, we need to jump to that note
    • Scroll the anchored note into view after notes rendered if page has note anchor
    • "Add reaction" needs to be hidden when signed out
    • Options menu and "Report as abuse" need to be hidden when signed out
    • "Report as abuse" should not be shown for comments by current user when signed in
    • "Preview" pane should say "Nothing to preview" when textarea is empty
    • "Comment" button should be disabled when textarea is empty
    • New notes and note edits don't have preview
    • "Toggle discussion" arrow doesn't flip
    • ESC doesn't cancel a note edit or discussion reply (immediately cancels when textarea is empty, and shows confirmation alert otherwise)
    • Show register and sign in links instead of comment when signed out
    • Fix autocomplete bug, request returns HTML and it causes recursive errors on the page
    • Fix slash command only comment
    • Fix syntax highlighting after editing a comment
    • When issue is closed/reopened from the issue actions at the top of the page, it should properly update close/reopen button state on the main note form at the bottom of the page
    • Handle quick actions notes.js:285
    • Fix visual bugs new note form, note type dropdown
    • Fix resurrecting autosaved comments into the reply widget
    • Make sure that we show first class action errors properly
    • Bottom padding of discussion reply "Comment" button is inconsistent with MR page
    • Discussion reply form should say "Markdown and quick actions are supported", not just "Markdown is supported" (only when adding a reply, not when editing)
    • When issue state changed using a slash command, we don't update the issue state buttons on the new note form
    • :100: emoji bug
    • Polling removes awards
    • Zen mode editing shows comment toolbar SS
    • Keyboard shortcuts
  • Technical Debts - [ ] All components starts with issue_note prefix and all of them under notes/components folder. Most of them should be moved into shared components folder and issue_ prefix should be removed
    • Use constants in Vuex
    • Separate state, getters, actions, mutations in Vuex store to their own files
Edited by Jacob Schatz

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Douwe Maan added 1 commit

    added 1 commit

    • 71cad7c8 - Add full JSON endpoints for issue notes and discussions

    Compare with previous version

  • @fatihacet I added the /:namespace/:project/issues/:iid/discussions.json endpoint that returns a list of all of the issue's discussions. Discussions with individual_note: true have a notes array with a single item. This "individual note discussion" represents a regular note on the issue itself, outside of a discussion. Discussions with individual_note: false are actual discussions, with at least 1 item in notes. This endpoint is the most appropriate to use for the initial load of the issue's notes/discussions.

    It also adds a full_data=true param to the /:namespace/:project/noteable/issue/:id/notes.json?full_data=true endpoint that returns a list of all the issue's notes, not grouped into discussions. Notes that are part of discussions are identified by type: "DiscussionNote" and can be grouped by discussion_id. Regular notes will have type: null. This endpoint is used by the polling mechanism, so any new notes will show up in this stream without the context of the discussion around them. Based on the type and discussion_id, you can determine what existing discussion the note should be added to.

    The full_data param also exists on the endpoints to create, update or destroy a note.

    Edited by Douwe Maan
  • Douwe Maan added 1 commit

    added 1 commit

    • af582d90 - Add full JSON endpoints for issue notes and discussions

    Compare with previous version

  • Douwe Maan added 1 commit

    added 1 commit

    • 3e3242d8 - Add full JSON endpoints for issue notes and discussions

    Compare with previous version

  • Douwe Maan added 1 commit

    added 1 commit

    • d3609ef4 - Add full JSON endpoints for issue notes and discussions

    Compare with previous version

  • @fatihacet Some things in the current discussion and note partials depend on note.user_authored?(current_user), which returns true if the current user is the author of the note. I didn't include this in the JSON at this point. Does the frontend have access to the current user ID otherwise? That would of course be enough to verify this on the frontend.

  • Douwe Maan added 1 commit

    added 1 commit

    • 544f36b4 - Add data-discussions-path to issues notes div

    Compare with previous version

  • Author Contributor

    @DouweM we have gon.current_user_id. As we discussed, it should work.

  • Author Contributor

    @DouweM I checked the JSON and it looks very good. It has permissions, list of emojis, all necessary ids and paths etc. I can't think of a missing thing right now but of course there would be. For now it seems great. Thanks for doing this quickly, that JSON means a lot of work for me :joy:

    Edited by Fatih Acet
  • Fatih Acet added 1 commit

    added 1 commit

    • 18ae1c5d - Initial version of main component, Vuex store and service of issue discussions refactor.

    Compare with previous version

  • Author Contributor

    @DouweM this MR will be huge on the frontend side and it includes a new concept which is Vuex. For the MR widget refactor, we did incremental reviews and merged chunks into a review branch. I think we should do the same for this feature too. WDYT?

  • Filipa Lacerda
  • Filipa Lacerda
  • Luke Bennett
  • Author Contributor

    @DouweM I think the JSON is missing can_delete flag. If it's something client can know using the existing data, could you tell me how I can do it?

  • @fatihacet IIRC can_delete == can_edit.

  • Fatih Acet added 1 commit

    added 1 commit

    • a18d43d8 - Implement initial version of Vue notes for issues. :tada: :tada:

    Compare with previous version

  • Douwe Maan added 145 commits

    added 145 commits

    • a18d43d8...a1695253 - 139 commits from branch master
    • ce9fbef8 - Add Vuex as a dependency.
    • fae43b2c - Notes bundle for the issue discussions refactor.
    • 01c7ef04 - Add full JSON endpoints for issue notes and discussions
    • 1c0c847d - Add data-discussions-path to issues notes div
    • a2fda0cb - Initial version of main component, Vuex store and service of issue discussions refactor.
    • d1ab9dde - Implement initial version of Vue notes for issues. :tada: :tada:

    Compare with previous version

  • Fatih Acet marked the checklist item Render single note as completed

    marked the checklist item Render single note as completed

  • Fatih Acet marked the checklist item Render discussion as completed

    marked the checklist item Render discussion as completed

  • Fatih Acet marked the checklist item Implement toggle control for discussions as completed

    marked the checklist item Implement toggle control for discussions as completed

  • Fatih Acet marked the checklist item Show register and sign in links in discussion footer for anon user as completed

    marked the checklist item Show register and sign in links in discussion footer for anon user as completed

  • Fatih Acet changed the description

    changed the description

  • Fatih Acet changed the description

    changed the description

  • Fatih Acet added 3 commits

    added 3 commits

    • 1e7fa55f - Fix some ESLint errors.
    • f2a74529 - Add missing button title.
    • 8d51349a - Implement note awards as a vue component.

    Compare with previous version

  • Fatih Acet added 1 commit

    added 1 commit

    • 359b83da - Add comments to complicated awardTitle method.

    Compare with previous version

  • Fatih Acet changed the description

    changed the description

  • Fatih Acet changed the description

    changed the description

  • Fatih Acet added 3 commits

    added 3 commits

    • 6908d611 - Implement canAward.
    • 74e74153 - MarkdownField: Add extra prop to make spacing classes optional.
    • 14bc50f3 - IssueNotesRefactor: Implement note edit widget.

    Compare with previous version

  • Fatih Acet marked the checklist item Render :thumbsup: and :thumbsdown: at the beginning of the list as completed

    marked the checklist item Render :thumbsup: and :thumbsdown: at the beginning of the list as completed

  • Fatih Acet marked the checklist item Disable adding :thumbsup: and :thumbsdown: to your own comment as completed

    marked the checklist item Disable adding :thumbsup: and :thumbsdown: to your own comment as completed

  • Fatih Acet marked the checklist item Create award emoji list as completed

    marked the checklist item Create award emoji list as completed

  • Fatih Acet marked the checklist item Fix syntax highlighting as completed

    marked the checklist item Fix syntax highlighting as completed

  • Fatih Acet changed the description

    changed the description

  • Fatih Acet marked the checklist item Fix KaTex rendering as completed

    marked the checklist item Fix KaTex rendering as completed

  • Fatih Acet changed the description

    changed the description

  • Fatih Acet changed the description

    changed the description

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading