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

[Gitlab] Implement updating comment instead of deleting and recreating #141

Closed
bmaehr opened this issue Mar 22, 2020 · 6 comments
Closed
Labels
awaiting release Merged but not currently in release version enhancement New feature or request

Comments

@bmaehr
Copy link

bmaehr commented Mar 22, 2020

Describe the Enhancement
Implement updating comment instead of deleting and recreating

Additional context
If deleting all comments and recreating them already approved comments get reset, discussions deleted and the MR poluted.

@bmaehr bmaehr added the bug Something isn't working label Mar 22, 2020
@bmaehr
Copy link
Author

bmaehr commented Mar 22, 2020

#142

@mc1arke mc1arke added enhancement New feature or request and removed bug Something isn't working labels May 9, 2020
@bmaehr
Copy link
Author

bmaehr commented Mar 8, 2021

I have reimplemented the change based on master if someone is interested ...

mc1arke added a commit that referenced this issue Jul 7, 2021
The Gitlab decorator currently deletes all non-system comments posted by the user that Sonarqube is connected as and then creates comments for any issues reported in the current analysis, even where an issue is the same across multiple analyses of a Merge Request so would have an identical comment re-created under a new discussion. This causes issues with orphaned system comments - such as `UserXXX changed this line in version X of the diff` - and user discussions referring to Sonarqube's finding that no longer have the finding in the thread.

The decorator will now attempt to parse the comment's issue ID from the SonarQube link included in each comment and will close any discussion where the ID no longer exists in the analysis results, unless another user has also commented on the discussion in which case the decorator will comment that the issue is resolved but will leave the discussion open for manual review. Where the ID is not parsable a warning is logged and the discussion is left open.

To simplify the code within the decorator, the interactions with the Gitlab API have been pulled out into a separate GitlabClient, thereby allowing consistent authentication and error handling for Gitlab calls.
mc1arke added a commit that referenced this issue Jul 7, 2021
The Gitlab decorator currently deletes all non-system comments posted by the user that Sonarqube is connected as and then creates comments for any issues reported in the current analysis, even where an issue is the same across multiple analyses of a Merge Request so would have an identical comment re-created under a new discussion. This causes issues with orphaned system comments - such as `UserXXX changed this line in version X of the diff` - and user discussions referring to Sonarqube's finding that no longer have the finding in the thread.

The decorator will now attempt to parse the comment's issue ID from the SonarQube link included in each comment and will close any discussion where the ID no longer exists in the analysis results, unless another user has also commented on the discussion in which case the decorator will comment that the issue is resolved but will leave the discussion open for manual review. Where the ID is not parsable a warning is logged and the discussion is left open.

To simplify the code within the decorator, the interactions with the Gitlab API have been pulled out into a separate GitlabClient, thereby allowing consistent authentication and error handling for Gitlab calls.
mc1arke added a commit that referenced this issue Jul 7, 2021
The Gitlab decorator currently deletes all non-system comments posted by the user that Sonarqube is connected as and then creates comments for any issues reported in the current analysis, even where an issue is the same across multiple analyses of a Merge Request so would have an identical comment re-created under a new discussion. This causes issues with orphaned system comments - such as `UserXXX changed this line in version X of the diff` - and user discussions referring to Sonarqube's finding that no longer have the finding in the thread.

The decorator will now attempt to parse the comment's issue ID from the SonarQube link included in each comment and will close any discussion where the ID no longer exists in the analysis results, unless another user has also commented on the discussion in which case the decorator will comment that the issue is resolved but will leave the discussion open for manual review. Where the ID is not parsable a warning is logged and the discussion is left open.

To simplify the code within the decorator, the interactions with the Gitlab API have been pulled out into a separate GitlabClient, thereby allowing consistent authentication and error handling for Gitlab calls.
mc1arke added a commit that referenced this issue Jul 7, 2021
The Gitlab decorator currently deletes all non-system comments posted by the user that Sonarqube is connected as and then creates comments for any issues reported in the current analysis, even where an issue is the same across multiple analyses of a Merge Request so would have an identical comment re-created under a new discussion. This causes issues with orphaned system comments - such as `UserXXX changed this line in version X of the diff` - and user discussions referring to Sonarqube's finding that no longer have the finding in the thread.

The decorator will now attempt to parse the comment's issue ID from the SonarQube link included in each comment and will close any discussion where the ID no longer exists in the analysis results, unless another user has also commented on the discussion in which case the decorator will comment that the issue is resolved but will leave the discussion open for manual review. Where the ID is not parsable a warning is logged and the discussion is left open.

To simplify the code within the decorator, the interactions with the Gitlab API have been pulled out into a separate GitlabClient, thereby allowing consistent authentication and error handling for Gitlab calls.
mc1arke added a commit that referenced this issue Jul 7, 2021
The Gitlab decorator currently deletes all non-system comments posted by the user that Sonarqube is connected as and then creates comments for any issues reported in the current analysis, even where an issue is the same across multiple analyses of a Merge Request so would have an identical comment re-created under a new discussion. This causes issues with orphaned system comments - such as `UserXXX changed this line in version X of the diff` - and user discussions referring to Sonarqube's finding that no longer have the finding in the thread.

The decorator will now attempt to parse the comment's issue ID from the SonarQube link included in each comment and will close any discussion where the ID no longer exists in the analysis results, unless another user has also commented on the discussion in which case the decorator will comment that the issue is resolved but will leave the discussion open for manual review. Where the ID is not parsable a warning is logged and the discussion is left open.

To simplify the code within the decorator, the interactions with the Gitlab API have been pulled out into a separate GitlabClient, thereby allowing consistent authentication and error handling for Gitlab calls.
mc1arke added a commit that referenced this issue Jul 7, 2021
The Gitlab decorator currently deletes all non-system comments posted by the user that Sonarqube is connected as and then creates comments for any issues reported in the current analysis, even where an issue is the same across multiple analyses of a Merge Request so would have an identical comment re-created under a new discussion. This causes issues with orphaned system comments - such as `UserXXX changed this line in version X of the diff` - and user discussions referring to Sonarqube's finding that no longer have the finding in the thread.

The decorator will now attempt to parse the comment's issue ID from the SonarQube link included in each comment and will close any discussion where the ID no longer exists in the analysis results, unless another user has also commented on the discussion in which case the decorator will comment that the issue is resolved but will leave the discussion open for manual review. Where the ID is not parsable a warning is logged and the discussion is left open.

To simplify the code within the decorator, the interactions with the Gitlab API have been pulled out into a separate GitlabClient, thereby allowing consistent authentication and error handling for Gitlab calls.
mc1arke added a commit that referenced this issue Jul 8, 2021
The Gitlab decorator currently deletes all non-system comments posted by the user that Sonarqube is connected as and then creates comments for any issues reported in the current analysis, even where an issue is the same across multiple analyses of a Merge Request so would have an identical comment re-created under a new discussion. This causes issues with orphaned system comments - such as `UserXXX changed this line in version X of the diff` - and user discussions referring to Sonarqube's finding that no longer have the finding in the thread.

The decorator will now attempt to parse the comment's issue ID from the SonarQube link included in each comment and will close any discussion where the ID no longer exists in the analysis results, unless another user has also commented on the discussion in which case the decorator will comment that the issue is resolved but will leave the discussion open for manual review. Where the ID is not parsable a warning is logged and the discussion is left open.

To simplify the code within the decorator, the interactions with the Gitlab API have been pulled out into a separate GitlabClient, thereby allowing consistent authentication and error handling for Gitlab calls.
@mc1arke mc1arke added the awaiting release Merged but not currently in release version label Jul 11, 2021
@mc1arke
Copy link
Owner

mc1arke commented Aug 14, 2021

Released in v1.9.0 of the plugin

@mc1arke mc1arke closed this as completed Aug 14, 2021
mc1arke added a commit that referenced this issue Aug 22, 2021
The Gitlab decorator currently deletes all non-system comments posted by the user that Sonarqube is connected as and then creates comments for any issues reported in the current analysis, even where an issue is the same across multiple analyses of a Merge Request so would have an identical comment re-created under a new discussion. This causes issues with orphaned system comments - such as `UserXXX changed this line in version X of the diff` - and user discussions referring to Sonarqube's finding that no longer have the finding in the thread.

The decorator will now attempt to parse the comment's issue ID from the SonarQube link included in each comment and will close any discussion where the ID no longer exists in the analysis results, unless another user has also commented on the discussion in which case the decorator will comment that the issue is resolved but will leave the discussion open for manual review. Where the ID is not parsable a warning is logged and the discussion is left open.

To simplify the code within the decorator, the interactions with the Gitlab API have been pulled out into a separate GitlabClient, thereby allowing consistent authentication and error handling for Gitlab calls.

(cherry picked from commit 9d83cbe)
@mc1arke
Copy link
Owner

mc1arke commented Aug 23, 2021

Backported to 1.8.1 of the plugin for Sonarqube 8.9 support.

@felixoi
Copy link

felixoi commented Nov 2, 2021

@mc1arke Is there any chance to still delete the old summary or only update the first summary post every time? I'm using version 1.9.0 and the comment spam with summaries is insane when pushing some commits to a merge request.

@teake
Copy link
Contributor

teake commented Nov 23, 2021

@mc1arke Is there any chance to still delete the old summary or only update the first summary post every time? I'm using version 1.9.0 and the comment spam with summaries is insane when pushing some commits to a merge request.

@felixoi That bugged me too, so I've opened #494.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting release Merged but not currently in release version enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants