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

Add EE license check #151

Merged
merged 9 commits into from
Jun 15, 2020
Merged

Add EE license check #151

merged 9 commits into from
Jun 15, 2020

Conversation

larkox
Copy link
Contributor

@larkox larkox commented May 28, 2020

Summary

Add license check on plugin activate.

IMPORTANT: This require v5.24.0 of mattermost-server which is not yet released. Therefore, the package cannot be imported, and go.sum has not been properly updated. Probably this PR needs an update when v5.24.0 is in place.

Ticket Link

None

@larkox larkox added Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) 2: Dev Review Requires review by a core committer 1: PM Review Requires review by a product manager labels May 28, 2020
go.mod Outdated
@@ -7,7 +7,7 @@ require (
github.com/gorilla/mux v1.7.3
github.com/jarcoal/httpmock v1.0.4
github.com/mattermost/mattermost-plugin-api v0.0.9
github.com/mattermost/mattermost-server/v5 v5.3.2-0.20200313113657-e2883bfe5f37
github.com/mattermost/mattermost-server/v5 v5.24.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could some commit on master be used instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was discussed on a different conversation. We will point to master, merge this, and whenever 5.24 is addressable, we will update the plugin to address the correct server version.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@codecov-commenter
Copy link

codecov-commenter commented May 29, 2020

Codecov Report

Merging #151 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #151   +/-   ##
=======================================
  Coverage   23.51%   23.51%           
=======================================
  Files          62       62           
  Lines        2513     2513           
=======================================
  Hits          591      591           
  Misses       1848     1848           
  Partials       74       74           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63e1af8...ddc8321. Read the comment docs.

@larkox
Copy link
Contributor Author

larkox commented Jun 2, 2020

@aaronrothschild according to this conversation we discussed about checking for LDAP until we start using 5.24. Should that check be done in this PR?

@larkox larkox linked an issue Jun 2, 2020 that may be closed by this pull request
@larkox larkox added 3: QA Review Requires review by a QA tester and removed 2: Dev Review Requires review by a core committer labels Jun 8, 2020
@larkox larkox requested a review from DHaussermann June 8, 2020 08:28
@DHaussermann
Copy link

I tested this.
TE: Plugin fails to start
E0: Plugin fails to start
E10 : Plugin fails to start
E20: Plugin start fine
Based on code change only start up is affected so there is no need to regression test any specific functionality.

When it does not start the following is logged: You need a Enterprise License to activate this plugin

@aaronrothschild

  1. Can you just confirm that this will only work with E20 and not E10 as well (Just santiy check :) )
  2. given that is E10 only should we clarify the message in the log.

@larkox
Copy link
Contributor Author

larkox commented Jun 15, 2020

Regarding the log message, what about You need a Enterprise License (E20) to activate this plugin?

@aaronrothschild aaronrothschild removed the 1: PM Review Requires review by a product manager label Jun 15, 2020
Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed

  • This is working as expected.
  • Change suggestion above has been implemented

@DHaussermann DHaussermann removed the 3: QA Review Requires review by a QA tester label Jun 15, 2020
@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) labels Jun 15, 2020
@larkox larkox merged commit 7546d8b into mattermost:master Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Enterprise license check
7 participants