Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-18797 - Switch webapp to use '/plugins/marketplace' API to install… #3871

Merged
merged 5 commits into from
Oct 9, 2019
Merged

MM-18797 - Switch webapp to use '/plugins/marketplace' API to install… #3871

merged 5 commits into from
Oct 9, 2019

Conversation

alifarooq0
Copy link
Contributor

Summary

Switch webapp to use /plugins/marketplace API to install marketplace plugins

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-18797

Related Pull Requests

@alifarooq0 alifarooq0 added this to the v5.18.0 milestone Oct 4, 2019
@alifarooq0 alifarooq0 added the 2: Dev Review Requires review by a core commiter label Oct 4, 2019
marianunez
marianunez approved these changes Oct 4, 2019
View reviewed changes
Copy link
Member

@marianunez marianunez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@marianunez marianunez added the 3: QA Review Requires review by a QA tester label Oct 4, 2019
iomodo
iomodo reviewed Oct 7, 2019
View reviewed changes
components/plugin_marketplace/marketplace_item/marketplace_item.jsx
@@ -87,7 +87,7 @@ export default class MarketplaceItem extends React.Component {
this.setState({installing: true});
trackEvent('plugins', 'ui_marketplace_download');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we remove this? We are tracking in the redux, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as @ali-farooq0 suggested, we can know track it at the time of the api call given that we have the dedicated one for marketplace install.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marianunez
I'm personally OK keeping both, because once 5.16 is released, our analytics tool will contain this event. However >= 5.18 it'll be the other redux event, it might be hard to put together that data if you need to see how many people installed a marketplace plugin. My suggestion would to keep both now. If this change was going into 5.16 then it would've been fine removing that event.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guys, should we just merge this PR and I can follow up with Aaron later to fix the tracking events.
Follow up discussion: mattermost/mattermost-redux#938 (comment)

Created a jira to keep track: https://mattermost.atlassian.net/browse/MM-19318

@alifarooq0 alifarooq0 requested a review from iomodo October 9, 2019 13:04
iomodo
iomodo approved these changes Oct 9, 2019
View reviewed changes
Copy link
Contributor

@iomodo iomodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@marianunez marianunez requested a review from prapti October 9, 2019 14:37
@marianunez marianunez removed the 2: Dev Review Requires review by a core commiter label Oct 9, 2019
prapti
prapti approved these changes Oct 9, 2019
View reviewed changes
Copy link
Contributor

@prapti prapti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no new functionality to test as part of this ticket, only the tests are modified.

@prapti prapti removed the 3: QA Review Requires review by a QA tester label Oct 9, 2019
@alifarooq0 alifarooq0 merged commit e98c9d4 into plugin_signing Oct 9, 2019
@alifarooq0 alifarooq0 deleted the MM-18797 branch October 9, 2019 15:49
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Nov 19, 2019
lieut-data pushed a commit that referenced this pull request Nov 19, 2019
@alifarooq0 @lieut-data
MM-16368 - Plugin Signing (#4145)
bcd672b 
* MM-18797 - Switch webapp to use '/plugins/marketplace' API to install… (#3871)

* MM-18797 - Switch webapp to use '/plugins/marketplace' API to install marketplace plugins

* Updated tests

* Removing ui marketplace install event

* Revert "Removing ui marketplace install event"

This reverts commit 5419ce0.

* Updating redux dependency

* MM-19844 - Disallow plugin upload when PluginSettings.RequirePl… (#4109)

* MM-19844 - Disallow plugin upload when PluginSettings.RequirePluginSignature is true

* Fixed text

* Fixed snapshot

* Fixing snapshot

* Bumping redux ver

* temp redux commit

* update snapshots re: i18n

* Bumping redux ver
lieut-data pushed a commit that referenced this pull request Nov 19, 2019
@alifarooq0 @lieut-data
MM-16368 - Plugin Signing (#4145)
8751814 
* MM-18797 - Switch webapp to use '/plugins/marketplace' API to install… (#3871)

* MM-18797 - Switch webapp to use '/plugins/marketplace' API to install marketplace plugins

* Updated tests

* Removing ui marketplace install event

* Revert "Removing ui marketplace install event"

This reverts commit 5419ce0.

* Updating redux dependency

* MM-19844 - Disallow plugin upload when PluginSettings.RequirePl… (#4109)

* MM-19844 - Disallow plugin upload when PluginSettings.RequirePluginSignature is true

* Fixed text

* Fixed snapshot

* Fixing snapshot

* Bumping redux ver

* temp redux commit

* update snapshots re: i18n

* Bumping redux ver
@lindy65 lindy65 added the Tests/Not Needed Does not require new release tests label Nov 25, 2019
@sakshm-aurora sakshm-aurora mentioned this pull request Mar 27, 2024
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@marianunez marianunez marianunez approved these changes

@prapti prapti prapti approved these changes

@iomodo iomodo iomodo approved these changes

Assignees
No one assigned
Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Not Needed Does not require new release tests
Projects
None yet
Milestone
v5.18.0
6 participants
@alifarooq0 @prapti @marianunez @iomodo @lindy65 @amyblais