[GH-85,84,71,49] Confluence Phase-2 (Authorisation/Notifications) #89

Kshitij-Katiyar · 2022-07-22T09:55:34Z

Summary and Features Added:-

Removed the dependency on the Confluence-side plugin (OBR file) to support Confluence Server/Data Center subscriptions.
Added support for OAuth 2.0 for both Cloud and Server editions of Confluence. Users can OAuth using /Confluence connect command.
Remove the need to use a single shared authentication token limiting subscriptions to
admins only.
In the case of subscriptions, users can only subscribe to spaces and pages that their Confluence
account has access to, and the same for the events as well. Due to this, we had to change the way we were storing the subscription, and because of that the subscriptions were not backward compatible.
To tackle that we have added some commands to migrate the subscriptions and those commands can only be seen in the help command and auto-suggestion where there are some older subscriptions are present.
Support of the following events is there:-
• space_updated
• page_created
• page_restored
• page_trashed
• page_updated
• comment_created
• comment_updated
Added permissions for both Mattermost and confluence side which Mattermost Admin can change in Plugin configuration.

* MI-1721: WIP * MI-1721: WIP * MI-1721: Fixed GetClient function * MI-1721: WIP * [MI-1751]:Added Logic to create webhook for connecting to confluence server * MI-1721: Added OAuth2.0 for confluence server and cloud * MI-1721: Done some code refactoring * [MI-1772]:WIP:Working on Notifications * [MI-1751]:Fixed Review fixes * [MI-1751]:[WIP]:Added Notifications for server * [MI-1772]:[WIP]:refactoring code * [MI-1772]:Fixed Self Review fixes * [MI-1772]:Fixed Review fixes * [MI-1794][WIP]:Added Notifications for Confluence Cloud * [MI-1794]:Added Confluence Cloud Notification * [MI-1794]:Self review fixes * [MI-1788]:Completed Notifications for Confluence Server * [MI-1788]:Self Review Fixes * [MI-1811]:Fixed Unsubscribe command * [MI-1811]:Self review fixes * [MI-1814]:Added /edit Command in confluence Plugin * [MI-1794]:Fixed Review fixes * [MI-1822][WIP]:wrote some test cases * [MI-1826]:Added Permissions for who can create/edit/delete the subscriptions * [MI-1826]:Fixed review fixes * [MI-1788]:Fixed Review fixes * [MI-1811]:Fixed review fixes * [MI-1788]:Removed Kv store Admin Funtions * [MI-1814]:Fixed review fixes * [MI-1822]:Updated the test cases * [MI-1834][WIP]:Fixing bugs * [MI-1834]:self review fixes * [MI-1834]:review fixes done * [MI-1826]:review fixes done * [MI-1814]:review fixes done * [MI-1811]:review fixes done * [MI-1842]:Added test cases for Oauth flow * [MI-1842]:self review fixes * [MI-1842]:added yml files * [MI-1721]:fixed review fixes * [MI-1849]:fixed bug fixes * [MI-1849]:fixed templates * [MI-1849]:fixed review fixes * [MI-1853]:fixed linting errors * [MI-1853]:Fixed and removed lint erros * [MI-1853]:self review fixes * [MI-1853]:fixed linters in github actions * [MI-1853]:fixes goimports error * [MI-1853]:fix lint errors in webapp * [MI-1853]:fixed webapp test cases * [MI-1853]:Fixed circleCI error * [MI-1853]:fixed Circle lint error * [MI-1853]:Ran go mod tidy * [MI-1853]:fixed go.mod file * [MI-1853]:fixed go.mod and go.sum * [MI-1853]:fixed review fixes * [MI-1721]:fixed review fixes * [MI-1751]:Added Logic to create webhook for connecting to confluence server (#1) * [MI-1751]:Added Logic to create webhook for connecting to confluence server * [MI-1751]:Fixed Review fixes * [MI-1772]:Added logic for server side notifications for confluence (#3) * [MI-1772]:WIP:Working on Notifications * [MI-1751]:[WIP]:Added Notifications for server * [MI-1772]:[WIP]:refactoring code * [MI-1772]:Fixed Self Review fixes * [MI-1772]:Fixed Review fixes * [MI-1772]:fixed review fixes * [MI-1794]:Added Notifications for Confluence Cloud (#4) * [MI-1794][WIP]:Added Notifications for Confluence Cloud * [MI-1794]:Added Confluence Cloud Notification * [MI-1794]:Self review fixes * [MI-1794]:Fixed Review fixes * [MI-1794]:fixed review fixes * [MI-1794]:fixed review fixes * [MI-1788]:Created Notifications for Confluence Server(webhook per user) (#5) * [MI-1788]:Completed Notifications for Confluence Server * [MI-1788]:Self Review Fixes * [MI-1788]:Fixed Review fixes * [MI-1788]:Removed Kv store Admin Funtions * [MI-1811]:Added Logic for Unsubscribe Command (#6) * [MI-1811]:Fixed Unsubscribe command * [MI-1811]:Self review fixes * [MI-1811]:Fixed review fixes * [MI-1811]:review fixes done * [MI-1814]:Added /edit Command in confluence Plugin (#7) * [MI-1814]:Added /edit Command in confluence Plugin * [MI-1814]:Fixed review fixes * [MI-1814]:review fixes done * [MI-1814]:fixed review fixes * [MI-1822]:Updated the Test cases for Confluence Plugin (#9) * [MI-1822][WIP]:wrote some test cases * [MI-1822]:Updated the test cases * [MI-1822]:fixed review fixes * [MI-1822]:fixed review fixes Co-authored-by: Kshitij Katiyar <[email protected]> Co-authored-by: Kshitij Katiyar <[email protected]> Co-authored-by: Kshitij Katiyar <[email protected]> Co-authored-by: Kshitij Katiyar <[email protected]> Co-authored-by: Kshitij Katiyar <[email protected]> Co-authored-by: Kshitij Katiyar <[email protected]> Co-authored-by: Kshitij Katiyar <[email protected]> Co-authored-by: Shivam Chauhan <[email protected]> * [MI-1721]:fixed circle error * [MI-1721]:fixed frontend linters Co-authored-by: Kshitij Katiyar <[email protected]> Co-authored-by: kshitij katiyar <[email protected]>

mickmister

Thanks for this @Kshitij-Katiyar 👍

I've reviewed quite a bit of the code and still have about half the files to review. I've left some comments on what I've reviewed so far. Please let me know your thoughts.

.github/workflows/ci.yml

.golangci.yml

package-lock.json

mickmister · 2022-09-05T21:40:02Z

webapp/src/actions/subscription_modal.js

@@ -41,6 +41,28 @@ export const editChannelSubscription = (body) => {
    };
 };

+export function getConnected() {


Seems weird to put this in this file but there's no "general" actions file so I suppose this makes sense. I wonder if we'll be adding other actions files in the future though

Should I create a new general file or is this fine?

mickmister · 2022-09-05T22:02:30Z

server/serializer/channel_subscription.go

-	ByURLPagID    map[string]StringArrayMap
-	ByURLSpaceKey map[string]StringArrayMap
+	ByURLPageID   map[string]StringStringArrayMap
+	ByURLSpaceKey map[string]StringStringArrayMap


Changed data type here too. Is this backwards compatible?

@mickmister no it is not but i am working on a feature to migrate the older subscriptions to the newer version of confluence

server/serializer/confluence_cloud.go

server/serializer/confluence_server.go

mickmister · 2022-09-06T21:20:20Z

My main concern is saving the configuration for each Confluence server in the config. I'm thinking this should instead be in the KV store and be configured with slash commands or a new Flow, rather than adding a custom setting for the admin console.

There are a lot of benefits to this, including being able to configure these things with the Mattermost mobile app.

javaguirre · 2022-10-17T09:16:53Z

In case of the command.go file, it's better if you start changes and you move it so Git gets that it was the same file, I haven't checked the commits, but for some reason I think the file was deleted in a commit and then added in another, so we can't see the diff there. I have used a online diff tool to see better those changes.

https://www.diffchecker.com/wNVsBTgB

javaguirre · 2022-10-17T11:40:54Z

I don't understand well the difference between this PR and #88 . I think there is a very big commit in common, so I'd need to review the same code twice, which is a bit inefficient, Could this PR have the #88 as base so we don't have the same changes in both? Or how were these PRs split into two?

Thank you! :-)

Kshitij-Katiyar · 2022-10-17T12:12:46Z

I don't understand well the difference between this PR and #88 . I think there is a very big commit in common, so I'd need to review the same code twice, which is a bit inefficient, Could this PR have the #88 as base so we don't have the same changes in both? Or how were these PRs split into two?

Thank you! :-)

@javaguirre PR 88 contains the code of this PR and 1 additional feature of creating a confluence page using mattermost. Actually, it was a concern of @mickmister that it would be better to break the PR 88 into 2 different PRs so I did exactly that. One of the broken-up PR is this PR 89 and I was not able to create the PR for the second feature due to the fact that 2nd feature also requires the code of this PR and I can't make incremental PRs on mattermost. Please ping freely in case of any query
Thank you! :-)

javaguirre · 2022-10-25T09:28:07Z

If you put PR #88 as a base for PR #89?

To be honest, I don't think we can effectively review this PR nor the #88 , It's too long and we can't really test even a good percentage of the features.

Next time would be nice having different PRs for different issues, having another PR as base different from master, but PRs longer than 500/750 LOC are almost impossible to review effectively.

I'd say we either accept this PR as is or close it, but I don't find a way of reviewing all these changes. I'll accept it, but next time I suggest creating smaller PRs that are related between them or trying to make independent changes thinking before about how to split the work.

Thank you!

server/client_server.go

server/edit_subscription.go

server/utils/utils.go

…#1) (#6) * [MI-2252]:Added Config manipulation through slash commands * [MI-2252]:Fixed review fixes * [MI-2252]:Fixed review fixes * [MI-2252]:fixed review fixes * [MI-2252]:Fixed refresh token bug * [MI-2261]:Created delete sub command for confluence config (#2) * [MI-2261]:Created delete sub command for confluence config * [MI-2261]:Fixed lint errors * [MI-2261]:fixed review fixes * [MI-2261]:Fixed review fixes * [MI-2261]:Fixed review fixes * [MI-2267]:Wrote test cases for adding configs through slash commands (#3) * [MI-2267]:Wrote test cases for adding confligs through slash commands * [MI-2267]:Added default url for config modal * [MI-2267]:Fixed review fixes * [MI-2267]:Fixed review fixes * [MI-2267]:Fixed self review fixes * [MI-2267]:Fixed review fixes * [MI-2267]:fixed review fixes * [MI-2267]:fixed review fixes * [MI-2267]:Fixed review fixes * [MI-2280]:Add Dynamic Argument for config add command (#4) * [MI-2280]:Add Dynamic Argument for config add command * [MI-2280]:Fixed review fixes * [MI-2280]:Fixed CI error * [MI-2280]:Fixed CI errors * [MI-2280]:Fixed CI lint errors * [MI-2280]:Fixed test cases * [MI-2280]:fixed CI error * [MI-2280]:fixed review fixes

Kshitij-Katiyar · 2022-11-03T08:12:44Z

@mickmister Added the feature for config manipulation through slash commands and not using the custom component

Kshitij-Katiyar · 2022-11-15T06:19:31Z

@mickmister Added feature to migrate old confluence subscriptions to this newer version

javaguirre

I didn't want to block this and Michael has much more context, but now that I have time I'd like to discuss some changes/improvements that won't take much time.

Thank you!

server/serializer/channel_subscription.go

server/serializer/confluence_config.go

server/serializer/page_subscription.go

server/test_utils.go

server/utils/utils.go

* [MI-2362]:Fixed review fixes of confluence PR 89 on mattermost * [MI-2362]:Fixed review fixes

* [MI-2366]:Fixed review fixes of confluence PR 88 on mattermost * [MI-2366]:Fixed CI error * [MI-2366]:Fixed review fixes * [MI-2366]:Fixed review fixes

Kshitij-Katiyar · 2022-11-18T12:06:45Z

@javaguirre I Fixed review comments from PR 88 and 89 and I have pushed 2 separate commits for review comments for different branches.

javaguirre · 2022-11-18T12:13:15Z

@javaguirre I Fixed review comments from PR 88 and 89 and I have pushed 2 separate commits for review comments for different branches.

Thank you for the effort! I'll review it right away! 💯

javaguirre

I don't see anything else for now, but I'll review it again next week.

server/kv.go

* bump version to 1.3.0 (mattermost-community#97) * [MI-2413]:Fixed review comments given by mm team * [MI-2413]:Fixed merge conflicts * [MI-2413]:Fixed lint errors * [MI-2413]:Fixed go critic error * [MI-2413]:Fixed manifest.go error * [MI-2413]:Fixed review fixes * [MI-2413]:Fixed review fixes * [MI-2413]:fixed build error Co-authored-by: Michael Kochell <[email protected]>

javaguirre

I don't want to block this more than necessary.

This PR needs special attention regarding QA, especially for backward compatibility issues, and support after releasing a new version of the plugin.

Kshitij-Katiyar · 2022-12-12T08:38:21Z

I don't want to block this more than necessary.

This PR needs special attention regarding QA, especially for backward compatibility issues, and support after releasing a new version of the plugin.

@javaguirre Sure let me know if you need anything

* [MI-2449]:Fixed bugs found by QA * [MI-2449]:Fixed lint error * [MI-2449]:Fixed layout of autosuggest in commands * [MI-2449]:Fixed review fixes * [MI-2449]:Fixed test cases * [MI-2449]:Fixed lint errors * [MI-2449]:Added proper error * [MI-2449]:Fixed connect command error * [MI-2449]:Trim the url in the interactive dialogue * [MI-2449]:Fixed review fixes * [MI-2483]:Fixed the URL error in config modal (#17) * [MI-2483]:Fixed the URL error in config modal * [MI-2483]:Fixed review fixes * [MI-2483]:Fixed lint errors * [MI-2449]

raghavaggarwal2308 · 2024-01-22T13:56:01Z

@mickmister Do we need to add this PR & PR #88 in an upcoming release?

mickmister · 2024-01-22T16:21:30Z

@raghavaggarwal2308 Sounds good to me 👍

…luence into auth/notification

* Updated the readme * fixed review comments * Fixed lint errors

raghavaggarwal2308 · 2024-02-13T11:12:21Z

@mickmister Gentle reminder to re-review this PR

…luence into auth/notification

raghavaggarwal2308 · 2024-04-25T11:16:36Z

@mickmister Gentle reminder to re-review

shivamjosh and others added 11 commits July 21, 2022 14:25

Reverted unnecessary Makefile changes

e9cb0f4

fixing bugs

7674a87

fixing merge conflicts

872fcc0

[MI-1897]:bug fix

847e853

[MI-1897]:fixed github workflow

e5f3894

[MI-1897]:fixed review fixes

4acd7f4

[MI-1897]:fixed review fixes

ed4758a

added checks for commands

5ec8256

fixed test cases

62d3cab

Updated assets

dbabb8b

Kshitij-Katiyar requested a review from jfrerich as a code owner July 22, 2022 09:55

Kshitij-Katiyar mentioned this pull request Jul 22, 2022

[GH-85,84,71,49] (WIP)Confluence Phase-2 #88

Draft

catalintomai requested review from mickmister and catalintomai July 26, 2022 03:11

jfrerich removed their request for review July 27, 2022 18:37

mickmister requested changes Sep 6, 2022

View reviewed changes

Updated userID to delete subscription

053b9d4

catalintomai requested review from javaguirre and removed request for catalintomai October 14, 2022 16:11

javaguirre suggested changes Oct 25, 2022

View reviewed changes

server/client_server.go Outdated Show resolved Hide resolved

server/edit_subscription.go Show resolved Hide resolved

server/utils/utils.go Outdated Show resolved Hide resolved

Kshitij-Katiyar requested a review from catalintomai as a code owner October 31, 2022 13:25

Kshitij-Katiyar mentioned this pull request Nov 3, 2022

[MI-2324]:Fixed review fixes of confluence auth/notification PR Brightscout/mattermost-plugin-confluence-fork#10

Merged

hanzei mentioned this pull request Nov 14, 2022

Release v1.3.0 #95

Closed

javaguirre suggested changes Nov 15, 2022

View reviewed changes

Kshitij-Katiyar added 2 commits November 18, 2022 17:28

[MI-2362]:Fixed review fixes of confluence PR 89 on mattermost (#13)

3d4f6c0

* [MI-2362]:Fixed review fixes of confluence PR 89 on mattermost * [MI-2362]:Fixed review fixes

[MI-2366]:Fixed review fixes of confluence PR 88 on mattermost (#12)

e843a42

* [MI-2366]:Fixed review fixes of confluence PR 88 on mattermost * [MI-2366]:Fixed CI error * [MI-2366]:Fixed review fixes * [MI-2366]:Fixed review fixes

Kshitij-Katiyar requested review from javaguirre and removed request for mickmister November 18, 2022 12:04

deleted github workflow files

debb47f

javaguirre reviewed Nov 18, 2022

View reviewed changes

javaguirre suggested changes Nov 21, 2022

View reviewed changes

server/kv.go Outdated Show resolved Hide resolved

server/kv.go Show resolved Hide resolved

server/kv.go Outdated Show resolved Hide resolved

server/kv.go Outdated Show resolved Hide resolved

Kshitij-Katiyar requested a review from javaguirre December 2, 2022 11:33

javaguirre approved these changes Dec 5, 2022

View reviewed changes

hanzei requested a review from mickmister February 15, 2023 11:51

hanzei removed the request for review from mickmister May 30, 2023 19:10

mickmister self-requested a review January 22, 2024 16:21

ayusht2810 and others added 2 commits February 8, 2024 17:13

Merge branch 'master' of github.com:mattermost/mattermost-plugin-conf…

789389f

…luence into auth/notification

Updated the readme to include the new features (#19)

8650c09

* Updated the readme * fixed review comments * Fixed lint errors

raghavaggarwal2308 and others added 3 commits February 19, 2024 18:24

Merge branch 'master' of github.com:mattermost/mattermost-plugin-conf…

88e9fdc

…luence into auth/notification

Removed unused files

3312e4d

Fixed merge conflicts

3c2e771

mickmister requested a review from AayushChaudhary0001 July 10, 2024 03:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GH-85,84,71,49] Confluence Phase-2 (Authorisation/Notifications) #89

[GH-85,84,71,49] Confluence Phase-2 (Authorisation/Notifications) #89

Kshitij-Katiyar commented Jul 22, 2022 •

edited

Loading

mickmister left a comment

mickmister Sep 5, 2022

Kshitij-Katiyar Nov 3, 2022 •

edited

Loading

mickmister Sep 5, 2022

Kshitij-Katiyar Nov 3, 2022

mickmister commented Sep 6, 2022 •

edited

Loading

javaguirre commented Oct 17, 2022

javaguirre commented Oct 17, 2022 •

edited

Loading

Kshitij-Katiyar commented Oct 17, 2022 •

edited

Loading

javaguirre commented Oct 25, 2022

Kshitij-Katiyar commented Nov 3, 2022

Kshitij-Katiyar commented Nov 15, 2022

javaguirre left a comment

Kshitij-Katiyar commented Nov 18, 2022

javaguirre commented Nov 18, 2022

javaguirre left a comment

javaguirre left a comment

Kshitij-Katiyar commented Dec 12, 2022

raghavaggarwal2308 commented Jan 22, 2024 •

edited

Loading

mickmister commented Jan 22, 2024

raghavaggarwal2308 commented Feb 13, 2024

raghavaggarwal2308 commented Apr 25, 2024

[GH-85,84,71,49] Confluence Phase-2 (Authorisation/Notifications) #89

Are you sure you want to change the base?

[GH-85,84,71,49] Confluence Phase-2 (Authorisation/Notifications) #89

Conversation

Kshitij-Katiyar commented Jul 22, 2022 • edited Loading

mickmister left a comment

Choose a reason for hiding this comment

mickmister Sep 5, 2022

Choose a reason for hiding this comment

Kshitij-Katiyar Nov 3, 2022 • edited Loading

Choose a reason for hiding this comment

mickmister Sep 5, 2022

Choose a reason for hiding this comment

Kshitij-Katiyar Nov 3, 2022

Choose a reason for hiding this comment

mickmister commented Sep 6, 2022 • edited Loading

javaguirre commented Oct 17, 2022

javaguirre commented Oct 17, 2022 • edited Loading

Kshitij-Katiyar commented Oct 17, 2022 • edited Loading

javaguirre commented Oct 25, 2022

Kshitij-Katiyar commented Nov 3, 2022

Kshitij-Katiyar commented Nov 15, 2022

javaguirre left a comment

Choose a reason for hiding this comment

Kshitij-Katiyar commented Nov 18, 2022

javaguirre commented Nov 18, 2022

javaguirre left a comment

Choose a reason for hiding this comment

javaguirre left a comment

Choose a reason for hiding this comment

Kshitij-Katiyar commented Dec 12, 2022

raghavaggarwal2308 commented Jan 22, 2024 • edited Loading

mickmister commented Jan 22, 2024

raghavaggarwal2308 commented Feb 13, 2024

raghavaggarwal2308 commented Apr 25, 2024

Kshitij-Katiyar commented Jul 22, 2022 •

edited

Loading

Kshitij-Katiyar Nov 3, 2022 •

edited

Loading

mickmister commented Sep 6, 2022 •

edited

Loading

javaguirre commented Oct 17, 2022 •

edited

Loading

Kshitij-Katiyar commented Oct 17, 2022 •

edited

Loading

raghavaggarwal2308 commented Jan 22, 2024 •

edited

Loading