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

Enterprise demo (trial) process #107

Merged
merged 15 commits into from
Jul 6, 2017
Merged

Conversation

phisch
Copy link
Contributor

@phisch phisch commented Jul 5, 2017

This PR contains the UI and backend for the trial process feature. It requests the demo license key from the marketplace, saves the key in appconfig and installs all enterprise applications.

Additionally the enterprise_key app will be able to load enterprise keys from the appconfig if there is no key configured in config.php.

@@ -33,7 +33,8 @@
['name' => 'market#uninstall', 'url' => '/apps/{appId}/uninstall', 'verb' => 'POST'],
['name' => 'market#getApiKey', 'url' => '/apikey', 'verb' => 'GET'],
['name' => 'market#changeApiKey', 'url' => '/apikey', 'verb' => 'PUT'],

['name' => 'market#hasLicenseKey', 'url' => '/has-license-key', 'verb' => 'GET'],
['name' => 'market#requestDemoLicenseKeyFromMarket', 'url' => '/request-license-key-from-market', 'verb' => 'GET'],
Copy link
Member

Choose a reason for hiding this comment

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

something which is named request used GET ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, i really wasn't sure here, since it is not returning the subject itself, it makes sure the license key is available, and fetches it if not

Copy link
Member

Choose a reason for hiding this comment

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

well - GET shall not change the server state - this call does change the server state -> no GET

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does change the state of the server, but not by giving it information. It is basically a trigger for a background task. In this case GET, POST, PULL and PUSH are all wrong. GET seemed like the most sane solution.

Copy link
Member

Choose a reason for hiding this comment

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

for a trigger I'd use POST - because it's state changing

);

if (!array_key_exists('license_key', $data)) {
throw new MarketException('Marketplace did not return a demo license key.');
Copy link
Member

Choose a reason for hiding this comment

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

missing translation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it's not used to display in any userinterface.

Copy link
Member

Choose a reason for hiding this comment

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

but what is returned to the user interface then?
both cases throw market exception - kind of unclear .....

$demoLicenseKey = $data['license_key'];

if (!$demoLicenseKey) {
throw new MarketException('Marketplace returned an empty demo license key.');
Copy link
Member

Choose a reason for hiding this comment

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

missing translation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it's not used to display in any userinterface.

@@ -0,0 +1,11 @@
<?php
Copy link
Member

Choose a reason for hiding this comment

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

missing file header with license ....

Copy link
Contributor Author

@phisch phisch Jul 6, 2017

Choose a reason for hiding this comment

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

ty, fixed! also in the other exception class

@DeepDiver1975 DeepDiver1975 merged commit 10e951a into master Jul 6, 2017
@DeepDiver1975 DeepDiver1975 deleted the enterprise-trial-proce branch July 6, 2017 19:37
DeepDiver1975 pushed a commit that referenced this pull request Aug 14, 2017
* added route for license key check

* added functionallity to check if a license key is available, also added api action so the frontend can use it

* make sure existing market app logic uses the getLicenseKey method to also get keys from appconfig if necessary

* Add "Start trial" button and modal,
add Sourcemaps for easier js debugging

* Move API key settings to the menu,
make slicker trial form

* added exceptions

* added method to request and save the demo license key from the marketplace

* added api endpoint to load the demo license key from the marketplace

* fixed false appconfig key

* Improve EE Trial process [WIP]

* Fix license error

* fixed EnterpriseKey usage

* Finalise trial process

* added missing license header
DeepDiver1975 pushed a commit that referenced this pull request Aug 14, 2017
* added route for license key check

* added functionallity to check if a license key is available, also added api action so the frontend can use it

* make sure existing market app logic uses the getLicenseKey method to also get keys from appconfig if necessary

* Add "Start trial" button and modal,
add Sourcemaps for easier js debugging

* Move API key settings to the menu,
make slicker trial form

* added exceptions

* added method to request and save the demo license key from the marketplace

* added api endpoint to load the demo license key from the marketplace

* fixed false appconfig key

* Improve EE Trial process [WIP]

* Fix license error

* fixed EnterpriseKey usage

* Finalise trial process

* added missing license header
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants