Show related merge requests and issues as well as attachments as tabs

See gitlab-org/gitlab-ce#3180

Closes gitlab-org/gitlab-ce#3180

Title changed from Show related merge requests and issues as well as attachments as tabs to WIP: Show related merge requests and issues as well as attachments as tabs

mentioned in merge request !1771 (closed)

Added 1 commit:

• 4b93b44b - ensure tab header and sidebar only appear in discussion tab pane

Added 1 commit:

• cdb7b67c - remove unused CSS class. refine attachments to anything reference /uploads/ URLs.

/cc @DouweM @creamzy

Am I on the right track? Yes, there is more to go.

Added 1 commit:

• b1f0ed10 - standardize as mentioned_NOUNs

@creamzy I'm starting to think we should at least keep the row that has "3 participants ... gitlab-org/gitlab-ce#10" in all of the tabs.

Added 1 commit:

• 7d3363c7 - revert some unintended changes. put the issue participants div across all four tabs.

Added 1 commit:

• 1ac83f93 - hide checkbox when not in a list of issues

@gsmethells Thanks for all the work!

@dzaporozhets and @JobV you have to see this, more context is in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1771

@gsmethells Definitely on the right track, great job so far :)

@gsmethells looking great so far, much better than the first mockups. Keep it up!

@gsmethells awesome.

Added 3 commits:

• f2e9107c - no longer needed
• 88c72965 - fix up some spots based upon feedback
• 28eee85b - forgot one more render that needed show_checkbox

Added 1 commit:

• dc4980f8 - fix up some spots based upon feedback.

Added 1 commit:

• a4e7d672 - port changes to Merge Request page

Added 1 commit:

• a515e0cd - remove unused file

@creamzy @DouweM Since some references may come from the Discussion (entity.notes.each note.all_references) while others come from the Description (entity.all_references), how does the "Show in Discussion" behave if the reference came from the Description instead of the Discussion? Just elide the "Show in Discussion" entirely?

@creamzy Is the "Related Merge Requests" too long of a name for that tab in the Merge Requests page?

I think I have everything except the "Show in Discussion" part done from my checklist in gitlab-org/gitlab-ce#3180 and I am looking for some more feedback.

Honestly, I wouldn't mind breaking out the "Show in Discussion" feature proposal into its own separate issue.

Added 1 commit:

• ef8e7db7 - update the CHANGELOG

Deleted source branch all-references-lists

Restored source branch all-references-lists

Added 40 commits:

• 075e3661..98bdbd35 - 40 commits from branch gitlab-org:master

@DouweM the all_references methodology seems to only pick up on a reference if the other entity made the reference rather than the current entity.

For example, if gitlab-org/gitlab-ci#10 is mentioned in gitlab-org/gitlab-ce/merge_requests/1 as a comment, then the Issues tab in gitlab-org/gitlab-ce/merge_requests/1 will have 0 issues returned by mentioned_issues (as implemented above) rather than 1 for gitlab-org/gitlab-ci#10. Now, however, if I go to gitlab-org/gitlab-ci/issues/10 and mention gitlab-org/gitlab-ce!1 in a comment there and then return to the gitlab-org/gitlab-ce/merge_requests/1 page, finally the Issues tab now has 1 issue listed as expected.

If I get rid of .system in notes.system.flat_map, then this is fixed, it seems. What is .system doing here?

Added 1 commit:

• e61e55c4 - do uniq only once. remove system from map flattening.

mentioned in issue #3180 (closed)

Added 1 commit:

• b626c493 - no checkboxes in groups view of issues list.

Title changed from WIP: Show related merge requests and issues as well as attachments as tabs to Show related merge requests and issues as well as attachments as tabs

Added 1 commit:

• f8d803d4 - simplify link_to per gitlab_router_helper doc

Rebased (a little while ago).

Added 1 commit:

• 765718fa - nick comment

Added 1 commit:

• 7007a7a6 - typesetting

Added 1 commit:

• b635341e - DRY up the participants view

@gsmethells first of all I appreciate your passion about implementing this. But it has a core problem that should be solved.

The last screenshot reached 6(!) tabs - this does not work. I put veto on such implementation. Issue has only one valuable content holder - its discussion. Related merge requests, attachments etc is not EQUAL to whole discussion. In same way diff of merge request is absolutely much more valuable than list of related merge requests.

All this info with related issues, attachments, merge requests etc should not take a primary position in ui. This problem should be solved before any progress with implementation. I don't want contributors spend personal time on something that will not get merged. If @creamzy or @skyruler do not have ideas on how to make it nice - we will reject the whole idea. I rather have less functionality but keep UI focused on what matters.

@dzaporozhets the UI expanded when feedback to !1771 (closed) provided a list of suggestions. My original need is just to list the merge requests on the issue page and no more. I'd be happy leaving it there, but I would be unhappy to scrap it entirely.

The inability to see the status of related merge requests cross-project for an issue (especially for client-server architectures with supporting libraries all in different projects) is a hurdle to adoption.

With comments like the one from @sytses and @creamzy, I know there is value here to be had from the original need.

The simplified view (merge request page in this case is unchanged from its original 3 tabs):

@gsmethells thank you. I understand your basic need and I expect @creamzy just helps you with showing this mrs list somehow (can be tab or something else).

What I don't understand is how we end up with this - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1941#note_2828617?

Well that occurred from https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1771#note_2782876 which suggested displaying all cross-references. This was then given what felt like a go-ahead in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1771#note_2813768 which I was waiting for first.

The full discussion is in !1771 (closed) where the first attempts and feedback were given. This MR is based upon that feedback cycle.

@gsmethells thank you very much for providing links!

The https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1771#note_2788993 comment also felt like approval. Anyway, don't kill it yet. It has purpose for our company and I'm sure many others out there. It's a great project, just trying to help others while helping my own people be productive.

@gsmethells one last question please. Did you have screenshots in this MR before this comment - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1941#note_2823244? I see people commented with looking great but I see no screenshot above this comments.

@dzaporozhets @sytses I will wait for a go-ahead from one of you before resuming efforts.

Somehow this MR and !1771 (closed) was blessed by @DouweM @JobV @creamzy and @sytses. I really don't get how this happened. Does someone find it usable to have merge request page like this https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1941#note_2828617 ?

My original need is just to list the merge requests on the issue page and no more Anyway, don't kill it yet.

@gsmethells no problem. Now I know what you need and its a matter of collaboration with UX designer to see if we can make it nice and usable . Again thank you for working on this.

Yes, I did delete those (screenshot) comments. The discussion was getting long with screenshots and I wanted to redo them. The screenshots that I deleted are effectively equivalent to the Issue page ones that are still here. Only the Merge Request page screenshot is new since then. I did that work today, but there were some Issue page tweaks, hence the re-taking of screen shots for that.

Yes, I did delete those (screenshot) comments.

@gsmethells ok this explains a lot. Thanks :)

Well the 3rd and 2nd to last bullet points in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1771#note_2782876 suggested what you see in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1941#note_2828617 (or at least that is how I perceived what was requested). Not that I want to be throwing anyone under a bus because @DouweM is awesome and helpful. :) I suspect that cross-planet, open-source project governance is hard and it's probably that which then leads to miscommunication and miscommunication is pretty much the root of all problems.

miscommunication is pretty much the root of all problems.

I agree. I see several communication problems here:

• complicating things. If you (contributor) need only A - we should avoid giving you advices on B and C.
• upvoting work VS upvoting result. While we always appreciate contributors work - result is not always what is good for most of users. Sometimes people show appreciation but you don't know if its because the fact of contributing or result from code perspective or result from UI perspective. Maybe we should be more detailed in such feedback.

@creamzy please provide a prototype for issue page where you render list of related merge requests with CI status. I propose to render a list below issue description and avoid tabs

Reassigned to @creamzy

The simplest UI is probably what I've shown here, but I know that folks were against color usage when I used that as the conversation starting point previously. Color is the most concise expression of status in the least amount of space, hence why I choose it. That list can be implemented such that it is hidden when there are no merge requests. It is also responsive in that more bootstrap badges simply wrap if they hit the end of the div width.

Just my 2 cents. I'm pretty open to suggestion at this point. :)

ok, lets try with this 2 minimal layouts

I show Status /Name of MR/ Project this MR belongs to/ for now

Variant 1

Variant 2

and decide where to move next and what info is vital from your prospective

/cc @dzaporozhets @DouweM @JobV @sytses @gsmethells

Assignee removed

@creamzy thank you. Looks awesome! I personally prefer Variant 2 but open for opinions

@gsmethells what do you think? I think it solves our problem and looks really nice

I like Variant 2. It is focused and to the point. The status of the issue resolution is clear and what projects were changed is apparent.

Well, you beat me to the punch there (first post), but I'm glad you picked the same one. It takes up less space.

Though, I have to add that if the merge request count exceeded some limit, I think it should be like in "Changes" where you have to click to see more/all of them, in this case that would be to see all of the rows of merge requests.

Though, I have to add that if the merge request count exceeded some limit, I think it should be like in "Changes" where you have to click to see more/all of them, in this case that would be to see all of the rows of merge requests.

I rather don't do it. I don't think its often when one issue can contain 10+ related merge requests.

@creamzy thanks for doing that up! @dzaporozhets thanks for sticking with me here.

@gsmethells just make it simple. Render list if any - no extra logic etc

No? Okay, you're the expert here. I just thought I'd cover the scaling case now before we got too deep into again.

There is no problem in rendering bigger list if there are more related merge requests

Alright, so are you ready to approve moving forward with some coding work?

The best approach is to solve particular problem in simplest way . You need a list of related merge requests in the issue page. You have UI mockup. Just implement it and let the future decide if there are any problems with it

Yes I approve implementation from @creamzy Variant 2

Only for issue page. Only list of related merge requests (if any).

Okay, I'll contribute this in a new MR and link to it from here.

I'll keep the change as tiny as humanly possible.

@gsmethells thank you! You are the best

Question: do we want the UI for each merge request row to visually differentiate between build.status and merge_request.state both? The main reasons why the pairing would be important to a user are clearest when considering what a user would do with the information from those pairs:

1. passed and open => could be merged (user may choose to follow MR link to take action)
2. passed and merged => cannot be merged (user would choose to not follow MR link as action already occurred)
3. passed and closed => cannot be merged (user sees record of what happened)
4. canceled and merged => cannot be merged (user would choose to not follow MR link but may choose to investigate why merged when canceled occurred)
5. failed and merged => ditto for merged when failed occurred

There are other pairings, of course, but I thought I'd point this out early on.

Question: do I take the Variant 2 literally when it uses red for #1042 but not green for #93?

We could re-use part of the current merge requests view which has:

- if ci_commit
  = render_ci_status(ci_commit)
- if merge_request.merged?
  %span
    = icon('check')
    MERGED
- elsif merge_request.closed?
  %span
    = icon('ban')
    CLOSED

Is that advantageous?

mentioned in merge request !1991 (merged)

Status changed to closed

Moving on to !1991 (merged)

Anyone interested in seeing the new screenshots based upon the latest mockup can head over to !1991 (merged) and check it out now.

mentioned in commit e5f4f1d1

mentioned in commit 9888a972

mentioned in commit 2bc82130