@ziegenberg opened this Pull Request on September 28th 2021 Contributor

Description:

Adds a new console command marketplace:add-license-key which adds a marketplace license key (Fixes #18070)

This is the first draft. It does not implement any checks like it's done in the Marketplace API: https://github.com/matomo-org/matomo/blob/2ac2bc1aeaf8efce6bb9af92506311da99e1757f/plugins/Marketplace/API.php#L69-L95

I'd need some pointers for further implementing this, if checking the license key this is deemed necessary.

Review

@ziegenberg commented on September 28th 2021 Contributor

The failing checks do not seem to be related to my change.

@github-actions[bot] commented on October 13th 2021 Contributor

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@tsteur commented on October 13th 2021 Member

@ziegenberg I see this PR is in draft. Not sure if that was you? Is that ready for a review?

@ziegenberg commented on October 13th 2021 Contributor

Hi @tsteur,

I left this in the draft state because it is most certainly not ready yet and I wanted to discuss my implementation before going to review. I suspect that I have misinterpreted the workflow here...

@tsteur commented on October 13th 2021 Member

@ziegenberg this looked all good to me. I would maybe just rename the command to be set license key instead of add license key.

@ziegenberg commented on October 13th 2021 Contributor

So you don't want to add any checks for validity while setting the license key?

@ziegenberg commented on October 13th 2021 Contributor

I quickly made the suggested changes and rebased onto current 4.x-dev

@tsteur commented on October 13th 2021 Member

@ziegenberg it depends what you are using the command for and how quick things need to go as the validation will take a bit of time. We have an API that you can use like \Piwik\Plugins\Marketplace\Api::getInstance()->saveLicenseKey($licenseKey) and then it will validate the license key and also invalidate caches.

This API won't "remove/unset" the license key though. If an empty license key is provided, then you would need to call the API method deleteLicenseKey().

Overall this would be better indeed 👍

@ziegenberg commented on October 13th 2021 Contributor

@tsteur I added the checks using the API. Does this look good to you?

@tsteur commented on October 13th 2021 Member

Nice, that looks good and works nicely 👍 Thanks for that.

This Pull Request was closed on October 13th 2021
Powered by GitHub Issue Mirror