Tuleap 16 est là ! Assistez à l'événement virtuel le 17 octobre à 10h30. Inscrivez-vous ici !

    •  
      epic #28288 Revamp of Pull Request Comments
    Summary
    Revamp of Pull Request Comments
    Pull Request

    Epic overview

    The goal of this epic is to gather all the changes that were requested around pull request (PR) comments to make a single, consistent proposal. The original requirements were:

    • Being able to reply to comment
    • Being able to use markdown in comments
    • Being able to go to comments easily from Overview page to Changes
    • Being able to search for pull requests

    Strategy

    Pull Request is one of the legacy apps written in AngularJS whose official support ended in January 2022. The goal should be to remove AngularJS usage, hence not adding new features on existing AngularJS codebase.

    The new developments should be done using Vue3 as decided in the ADR

    However, we won't rewrite from scratch the whole app (too big), we will split it. The current app covers:

    • The display of pull request for a repository
    • The "Overview" page of one PR
    • The "Commit" page of one PR
    • The "Changes" page of one PR

    In order to limit the amount of changes, especially on Changes (the diff) part the solution that was spiked is to transform the management of comments into a WebComponent, independant of the framework that can be used in both AngularJS (for the Change vue) and in Vue3 (for future Overview).

    Hence, the top level approach is the following:

    • The display of pull request for a repository -> most of the logic change, should be rewritten (Vue3)
    • The "Overview" page of one PR -> all the logic change, should be rewritten (Vue3)
    • The "Commit" page of one PR -> no change, it's kept as is (AngularJS)
    • The "Changes" page of one PR -> most of the logic don't change, it's kept in AngularJS but with usage of a WebComponent for the display of comments.

    Detailed plan

    Reply to comments

    1. Update PR app routing using a 3rd party component (Navigo) instead of AngularJS router
      1. The goal is to be able to have some pages in AngularJS and some others in Vue3
    2. Replace the current management of comments by a WebComponent shared in Overview and Changes view
    3. Add reply to comments in the WebComponent (no markdown yet).
      1. Comments: no markdown, no preview, no "resolved", no edit
      2. Comments that are done on a given file/line are rendered with a link toward the comment in Change view
      3. A first reply to a top level comment creates a Thread. A new color is associated to the thread (the goal is to help to distinguish one thread from another).
        1. The color is taken from Tuleap palette
        2. Color is persisted and the information is added into the PR comments REST payload (so the colors are the same on overview & change)
        3. At the time of writing, the idea is to let the backend pick-up the color based on the number of threads for a PR (ie. the 3rd thread is created, it picks the 3rd color in the list). Random picking can be an alternative but takes less into account potential conflict or "too near" colors.
      4. Comments are deployed in Change & Overview pages. Those pages are still in AngularJS at this stage:
        1. AngularJS fetches raw data
        2. Processing of the data is done by an external module (ie not angular code)
        3. Rendering is done with the WebComponent
      5. Support of Reply in Overview is not mandatory at this stage. The changes on Overview should be done in least effort mode (as the page will be rewritten afterward). The least effort here is to have all comments (ie including threads' responses) visible. Otherwise one might overlook comments that would be only visible in Changes.

    Rewrite Overview app (pre requisite of support of markdown)

    Design choice. In order to limit the amount of work related to text/markdown management (was really time consuming for introduction of MD in artifacts):

    • All existing comments remains in text and won't be editable
    • All new comments will be in Markdown only
    • The initial description (now rendered as first comment of the PR) was already editable and will be switched to Markdown at first edit
    • At this stage there is no change in the data or backend (ie REST routes & payload remains the same)

    In order to support markdown, the Overview should be rewritten:

    1. A vue3 app have been instantiated in anglar component, spike the fact that the full overview page is in vue 3
      1. Page routing is done via nginx
      2. Tabs Commit and Changes are still managed by Angular, but the links are instantiated by vue app
      3. We know that we will have load between the three different tabs (Overview/Commit/Changes) it is expected
    2. Render everything that is ready only
      1. The comment empty state block
      2. The pullrequest read only properties: Author, pullrequest date, changes, last CI status, references
      3. ⚠️ check that crossref & associated popover are ok (loadTooltips() from @tuleap/tooltip)
    3. Add the hybrid comment for managing comments
    4. Add the description as first possible comment (⚠️ with tooltips references)
      1. The description can not be updated
      2. We can not start a thread on description
    5. Add reviewers and make them updatable
    6. Add tags and make them updatable
    7. Add management of "Merge"
    8. Add management of "Abandon"
    9. Add "Checkout" button
    10. Edit Title (modale)
    11. Edit of first comment (previously description) in text only without preview
    12. Add new comment on Overview (reuse of NewInlineCommentForm)
      1. Remove feature flag and use new Overview for every one

    Add support of markdown

    1. First comment is rendered in Markdown + Preview (⚠️ no breaking change in REST API related to format: no format = text, only one format supported: Markdown)
      1. At first edit of an existing PR description that was in text, there is a warning to inform that after save the content will be rendered using markdown.
      2. Code blocks should be rendered using syntax highlighting
    2. All comments are rendered in Markdown + Preview
      1. All existing comments are flagged as text and cannot be edited (forgeupgrade)
      2. All new comments are in markdown and editable
      3. This applies on Overview and Changes

    Management of "Resolved" threads

    1. This is a new data (⚠️ REST APIs to be modified)
    2. Display & interactions (button) are carried by the first comment
    3. It should be possible to submit a new thread as already Resolved
    4. a thread can go from Unresolved to Resolved as well as from Resolved to Unresolved
    5. Permission: anyone who can merge can toggle the "Resolve" button
    6. Expand / Collapse of comments (except the first one)
    7. Display “X unresolved” in the file switcher in Changes view
    8. Add “Show only unresolved” filter on Overview (no user preference is saved, on reload all changes are displayed)

    Rewrite of Pull request list

    There are functional changes here (eg Branches no longer displayed) the original view should be kept a little while "in case of $REGERESSION".

    1. Display Open pull requests (title, date, owner, reviewers, tags & nb unresolved)
    2. Allow to see Closed pull requests
    3. Sort PRs (Newest, …)
    4. Related to me
    5. Search (title, author, labels, dates, branches). It's not a filter, it's a search

    Final mock-up

    https://enalean.notion.site/Pull-Requests-5f5bbe87a7224366b8e426c70b43c29b

    https://www.figma.com/proto/it3BHQkoGPDLTPIV8V6i3h/Git---Pull-requests?page-id=0%3A1&node-id=16%3A3624&viewport=13%2C471%2C0.31&scaling=scale-down&starting-point-node-id=16%3A3624

    Progress
    Empty
    Empty
    Closed
    Details
    #28288
    Manuel Vacelet (vaceletm)
    2023-12-22 15:35
    2022-09-08 15:56
    Attachments
    Empty
    References

    Follow-ups

    User avatar
    • Description
      Something went wrong, the follow up content couldn't be loaded
      Only formatting have been changed, you should switch to markup to see the changes
    • Status changed from On going to Closed
    User avatar
    Thomas Gorka (tgorka)2023-04-14 13:46
    • Summary
      -Rewamp of Pull Request Comments 
      +Revamp of Pull Request Comments 
    User avatar
    • Description
      Something went wrong, the follow up content couldn't be loaded
      Only formatting have been changed, you should switch to markup to see the changes
    User avatar
    • Description
      Something went wrong, the follow up content couldn't be loaded
      Only formatting have been changed, you should switch to markup to see the changes
    User avatar
    • Description
      Something went wrong, the follow up content couldn't be loaded
      Only formatting have been changed, you should switch to markup to see the changes
    • Status changed from Planned to On going
    User avatar
    Joris MASSON (jmasson)2022-09-09 11:24

    Typos and minor adjustments


    • Description
      Something went wrong, the follow up content couldn't be loaded
      Only formatting have been changed, you should switch to markup to see the changes
    User avatar
    • Description
      Something went wrong, the follow up content couldn't be loaded
      Only formatting have been changed, you should switch to markup to see the changes