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

Jump to bottom

Fix redirect link not working after plugin activation #17654

Merged
merged 3 commits into from Jun 8, 2021
Merged

Fix redirect link not working after plugin activation #17654

merged 3 commits into from Jun 8, 2021

Conversation

Findus23
Copy link
Member

@Findus23 Findus23 commented Jun 8, 2021

reported in https://forum.matomo.org/t/i-get-an-error-when-i-click-to-activate-the-plugin/42173/2

It turns out unlike the regular "activate plugin" buttons, the ones shown after uploading a plugin zip don't have a redirectTo GET parameter set.

matomo/plugins/CorePluginsAdmin/templates/uploadPlugin.twig

Line 29 in 587cc39

<p>{{ 'Marketplace_StepPluginSuccessfullyInstalled'|translate(plugin.name, plugin.version) }}</p>

This should not matter as an empty value means redirecting to the plugin or theme list (which is what we want.

matomo/plugins/CorePluginsAdmin/Controller.php

Lines 469 to 476 in 792bc0c

$plugin = $this->pluginManager->loadPlugin($pluginName);
$actionToRedirect = 'plugins';
if ($plugin->isTheme()) {
$actionToRedirect = 'themes';
}
$this->redirectToIndex('CorePluginsAdmin', $actionToRedirect);

But #17345 added a section that reads the parameters, but forgot to set a default value for redirectTo in case it is missing, throwing the error reported on the forum

Review

  • Functional review done
  • Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@Findus23 Findus23 added the Needs Review PRs that need a code review label Jun 8, 2021
@diosmosis diosmosis added Bug For errors / faults / flaws / inconsistencies etc. Regression Indicates a feature used to work in a certain way but it no longer does even though it should. and removed Bug For errors / faults / flaws / inconsistencies etc. labels Jun 8, 2021
@diosmosis diosmosis added this to the 4.4.0 milestone Jun 8, 2021
@diosmosis diosmosis merged commit d129126 into 4.x-dev Jun 8, 2021
@diosmosis diosmosis deleted the fixed-redirect-plugin-activation branch June 8, 2021 20:22
@diosmosis
Copy link
Member

Thanks for the fix @Findus23!

@mattab mattab changed the title Fixed redirect plugin activation Redirect link not working after plugin activation Jul 26, 2021
@mattab mattab changed the title Redirect link not working after plugin activation Fix redirect link not working after plugin activation Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Needs Review PRs that need a code review Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Milestone
4.4.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Findus23 @diosmosis