Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sidebar header MR count should show assigned MRs instead of opened MRs #271

Closed
jdeamicis opened this issue Dec 22, 2021 · 14 comments · Fixed by #394
Closed

Sidebar header MR count should show assigned MRs instead of opened MRs #271

jdeamicis opened this issue Dec 22, 2021 · 14 comments · Fixed by #394
Labels
Type/Enhancement New feature or improvement of existing feature

Comments

@jdeamicis
Copy link

As of now I see that the MR counter in the sidebar (the one showing Your open merge requests) shows the MRs someone has opened (i.e. the MRs for which the user is the author).

I believe it would make more sense to have the assigned MRs (i.e. the ones for which a user is assignee), rather than the opened ones.

Or maybe it could be made configurable.

@mickmister
Copy link
Contributor

Hi @deandrade87, Are you interested in implementing this?

@mickmister mickmister added this to the Future Consideration milestone Jan 12, 2022
@mickmister mickmister added Help Wanted Community help wanted Up For Grabs Ready for help from the community. Removed when someone volunteers Type/Enhancement New feature or improvement of existing feature and removed Help Wanted Community help wanted Up For Grabs Ready for help from the community. Removed when someone volunteers labels Jan 12, 2022
@sibasankarnayak
Copy link
Contributor

@mickmister wanted to check on this, is someone working on this ?

if no wanted to check on the requirement. does this feature seeking to list only PR been assigned to user , instead of listing all the PR opened by user?

@mickmister
Copy link
Contributor

mickmister commented Apr 7, 2022

@hanzei Do you have thoughts on this? I would think that "My opened MRs" is what someone would mainly want to know about, but maybe conventions in GitLab are a bit different than GitHub.

It looks like the "assignments" category currently only uses issues and not MRs, so assigned MRs are neglected in the counts in general.

func (g *gitlab) GetYourAssignments(user *UserInfo) ([]*internGitlab.Issue, error) {
client, err := g.gitlabConnect(*user.Token)
if err != nil {
return nil, err
}
opened := stateOpened
scope := scopeAll
var result []*internGitlab.Issue
var errRequest error
if g.gitlabGroup == "" {
result, _, errRequest = client.Issues.ListIssues(&internGitlab.ListIssuesOptions{
AssigneeID: &user.GitlabUserID,
State: &opened,
Scope: &scope,
})
} else {
result, _, errRequest = client.Issues.ListGroupIssues(g.gitlabGroup, &internGitlab.ListGroupIssuesOptions{
AssigneeID: &user.GitlabUserID,
State: &opened,
Scope: &scope,
})
}
return result, errRequest

Maybe we should match the GitHub plugin's behavior and include issues and MRs for the assignments category? @deandrade87 How do you feel about this?

@mickmister
Copy link
Contributor

mickmister commented Apr 7, 2022

@sibasankarnayak Is there a reason you're wanting to pick up an issue that's not in the sprint planning board? I ask because there are a decent amount of open tickets in the "Todo" column.

@mickmister
Copy link
Contributor

mickmister commented Apr 7, 2022

The user connect welcome message states that the third category contains both merge requests and issues you are assigned to. @dipak-demansol Are you able to verify this is the case?

cc @hanzei

image

@sibasankarnayak
Copy link
Contributor

@sibasankarnayak Is there a reason you're wanting to pick up an issue that's not in the sprint planning board? I ask because there are a decent amount of open tickets in the "Todo" column.

@mickmister i picked it bymistake , but thought as worked on it so raised the PR

@hanzei
Copy link
Collaborator

hanzei commented Apr 8, 2022

Maybe we should match the GitHub plugin's behavior and include issues and MRs for the assignments category?

That sound like the optimal solution to me

@jdeamicis
Copy link
Author

@hanzei Do you have thoughts on this? I would think that "My opened MRs" is what someone would mainly want to know about, but maybe conventions in GitLab are a bit different than GitHub.

It looks like the "assignments" category currently only uses issues and not MRs, so assigned MRs are neglected in the counts in general.

func (g *gitlab) GetYourAssignments(user *UserInfo) ([]*internGitlab.Issue, error) {
client, err := g.gitlabConnect(*user.Token)
if err != nil {
return nil, err
}
opened := stateOpened
scope := scopeAll
var result []*internGitlab.Issue
var errRequest error
if g.gitlabGroup == "" {
result, _, errRequest = client.Issues.ListIssues(&internGitlab.ListIssuesOptions{
AssigneeID: &user.GitlabUserID,
State: &opened,
Scope: &scope,
})
} else {
result, _, errRequest = client.Issues.ListGroupIssues(g.gitlabGroup, &internGitlab.ListGroupIssuesOptions{
AssigneeID: &user.GitlabUserID,
State: &opened,
Scope: &scope,
})
}
return result, errRequest

Maybe we should match the GitHub plugin's behavior and include issues and MRs for the assignments category? @deandrade87 How do you feel about this?

So, on GitLab the main buttons where a user sees their tasks are:

  • issues assigned to the user (this is compatible with what the mattermost plugin does already)
  • Merge Requests assigned to the user or waiting for a review by the user
  • ToDos (which are usually generated whenever a user is mentioned or so). I believe the mattermost plugin is treating these correctly.

So, for the second category I would expect the same behaviour as GitLab's, namely MRs assigned + pending review, NOT the MRs opened by a user.

Maybe we should match the GitHub plugin's behavior and include issues and MRs for the assignments category?

That sound like the optimal solution to me

I would tend to disagree. Mattermost users most likely expect to see on the plugin the exact same numbers they see in their GitHub or GitLab systems. Therefore I would expect the two plugins for GitHab and GitLab to have different behaviours, depending on what the underlying systems do.

@catalintomai
Copy link

Triage agreed that @deandrade87 's suggestion is valid.

@wiersgallak
Copy link

Closing due to inactivity. The issue can be reopened with more interest from our community.

@hanzei hanzei removed this from the Future Consideration milestone Jun 28, 2023
@mickmister mickmister reopened this Jul 19, 2023
@mickmister
Copy link
Contributor

mickmister commented Jul 28, 2023

@mkdbns I've added this to the plugin maintenance board. This isn't particularly urgent, but has been brought up by a few customers recently https://community.mattermost.com/core/pl/9pdx4qfbob8d7rs6jbs8ffaooo. The task is to implement the LHS counters as they are shown in GitLab's UI, as described here #271 (comment)

@raghavaggarwal2308
Copy link
Contributor

raghavaggarwal2308 commented Aug 17, 2023

@mickmister I have gone through the discussion and I think it would be best to have four buttons here (As there are three buttons on Gitlab and the "Merge requests" button has two subparts):

  1. Merge Request Assigned: This will display all the PRs assigned to the user.
  2. Merge Requests Needing Review: This will display the PRs assigned to the user for review.
  3. Issues: This will display all the issues assigned to a user (Same as the Gitlab system). This is already implemented but currently, the name of this button is "Your Assignments", we will update it to "Issues".
  4. To-Do List: Will display all the To-Dos (Same as the Gitlab system). This is already implemented but currently, the name of this button is "Unread Messages", we will update it to "To-Do list".

We are also thinking to update the icons on the "Issues" and "To-Do List" buttons to match the icons on Gitlab.
Please let us know your opinions on this.

Buttons on Gitlab:
gitlab-tabs

Demo video with the updated names on buttons:
Demo-video

@mickmister
Copy link
Contributor

@raghavaggarwal2308 Awesome, this all LGTM 👍

@mickmister
Copy link
Contributor

@jdeamicis Do you agree with this direction?

raghavaggarwal2308 added a commit to Brightscout/mattermost-plugin-gitlab that referenced this issue Aug 22, 2023
…how assigned MRs instead of opened MRs (#39)

* [MI-3405] Get proper data in sidebar buttons:
1. Get assigned PRs instead of the user's PRs.
2. Updated the name of sidebar buttons.
3. Updated API path.
4. Updated the name of API functions.
5. Updated the name of unreads to todos in the code.

* [MI-3405] Updated name of variables and functions

* [MI-3405] Updated icons in sidebar

* [MI-3405] Updated documentation

* [MI-3405] Reverted package-lock file changes

* [MI-3405] Review fixes
mickmister pushed a commit that referenced this issue Dec 19, 2023
…ed MRs instead of opened MRs (#39) (#394)

* [MI-3405] Get proper data in sidebar buttons:
1. Get assigned PRs instead of the user's PRs.
2. Updated the name of sidebar buttons.
3. Updated API path.
4. Updated the name of API functions.
5. Updated the name of unreads to todos in the code.

* [MI-3405] Updated name of variables and functions

* [MI-3405] Updated icons in sidebar

* [MI-3405] Updated documentation

* [MI-3405] Reverted package-lock file changes

* [MI-3405] Review fixes
raghavaggarwal2308 added a commit to Brightscout/mattermost-plugin-gitlab that referenced this issue Feb 14, 2024
…how assigned MRs instead of opened MRs (#39) (mattermost#394)

* [MI-3405] Get proper data in sidebar buttons:
1. Get assigned PRs instead of the user's PRs.
2. Updated the name of sidebar buttons.
3. Updated API path.
4. Updated the name of API functions.
5. Updated the name of unreads to todos in the code.

* [MI-3405] Updated name of variables and functions

* [MI-3405] Updated icons in sidebar

* [MI-3405] Updated documentation

* [MI-3405] Reverted package-lock file changes

* [MI-3405] Review fixes
mickmister added a commit that referenced this issue Feb 20, 2024
* Remove check for client secret length (#419)

* support client secrets longer than 64 chars

* remove length check

* change client secret length check to assume at least 64 chars

* change wording of error message

* [GH-321]:Fixed issue #321 'Wrong notification in case of assigned prs' (#395)

Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>
Co-authored-by: ayusht2810 <ayush.thakur@brightscout.com>

* [MI-3405] Fix issues #271: Sidebar header MR count should show assigned MRs instead of opened MRs (#39) (#394)

* [MI-3405] Get proper data in sidebar buttons:
1. Get assigned PRs instead of the user's PRs.
2. Updated the name of sidebar buttons.
3. Updated API path.
4. Updated the name of API functions.
5. Updated the name of unreads to todos in the code.

* [MI-3405] Updated name of variables and functions

* [MI-3405] Updated icons in sidebar

* [MI-3405] Updated documentation

* [MI-3405] Reverted package-lock file changes

* [MI-3405] Review fixes

* [MM-42] Fix CI error: implicit memory aliasing (#429)

* [MI-3588] Fix issue: Image attachment breaking in comment notification (#406)

* [MI-3719] Send the users an ephemeral message if they try to connect their account via MM desktop app (#416)

* [MI-3719] Send the users an ephemeral message if they try to connect their account via MM desktop app

* [MI-3713] Handles the following cases as well for desktop app:
1. Connecting using the button from the teams sidebar.
2. Connecting using the button from RHS.

* [MI-3719] Review fixes

* [MI-3719] Review fixes

Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>

* [MI-3719] Fix lint error

---------

Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>

* Fix lint errors

* Update plugin version

---------

Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>
Co-authored-by: kshitij katiyar <90389917+Kshitij-Katiyar@users.noreply.github.com>
Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>
Co-authored-by: ayusht2810 <ayush.thakur@brightscout.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type/Enhancement New feature or improvement of existing feature
Projects
7 participants