Skip to content

Fix psgdpr menu URL query separator#242

Merged
Hlavtox merged 4 commits into
PrestaShop:devfrom
Codencode:fix-241--Wrong-POST-action-throws-error-in-PS-9.1
Jun 4, 2026
Merged

Fix psgdpr menu URL query separator#242
Hlavtox merged 4 commits into
PrestaShop:devfrom
Codencode:fix-241--Wrong-POST-action-throws-error-in-PS-9.1

Conversation

@Codencode

@Codencode Codencode commented Apr 23, 2026

Copy link
Copy Markdown
Contributor
Questions Answers
Description? This fixes the BO module configuration URL when BO tokens are disabled.
Previously, the page query parameter was always appended using &, which produced an invalid URL when no query string was present yet.

Example:
- before: /admin/improve/modules/manage/action/configure/psgdpr&page=dataConfig
- after: /admin/improve/modules/manage/action/configure/psgdpr?page=dataConfig

When a token is present, the existing behavior is preserved:
- /admin/improve/modules/manage/action/configure/psgdpr?_token=xxx&page=dataConfig

In addition to fixing the query separator for module configuration URLs, it also:
- fixes dataConsent.tpl and dataConfig.tpl form actions so they use the proper query separator depending on whether the base admin URL already contains a query string
- makes Psgdpr::getTokenFromAdminLink() return an empty string when TokenInUrls::isDisabled() is enabled, to avoid relying on a token that is not present in the URL
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #241 - PrestaShop/PrestaShop#41314
Sponsor company Codencode snc
How to test? 1. Go to Advanced Parameters -> Security
2. Disable BO tokens
3. Open a module
4. Submit the configuration page form

@ps-jarvis

This comment was marked as off-topic.

@clotairer

Copy link
Copy Markdown
Contributor

Thank you for this PR. I had the issue in 9.1 and it works fine.

@Hlavtox Hlavtox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI suggested that duplicate parameters can occur. :-)

What about this?

function setQueryParameter(url, key, value) {
  const u = new URL(url, window.location.origin);
  u.searchParams.set(key, value);
  return u.toString();
}

window.history.pushState({}, "", setQueryParameter(moduleAdminLink, "page", item));

@ps-jarvis

ps-jarvis commented May 5, 2026

Copy link
Copy Markdown

This pull request seems to contain new translation strings. I have summarized them below to ease up review:

  • Modules.Psgdpr.Admin ⚠️
    • Data visualization and automatic actions
    • Find here listed all personal data collected by PrestaShop and your installed modules.
    • These data will be used at 2 different levels :
    • Configure your checkboxes
    • Please customize your consent request messages in the dedicated fields below :

(Note: this is an automated message, but answering it will reach a real human)

@Codencode

Copy link
Copy Markdown
Contributor Author

AI suggested that duplicate parameters can occur. :-)

What about this?

function setQueryParameter(url, key, value) {
  const u = new URL(url, window.location.origin);
  u.searchParams.set(key, value);
  return u.toString();
}

window.history.pushState({}, "", setQueryParameter(moduleAdminLink, "page", item));

Ok, I made the change. I also created an additional commit, because I realized that we also needed to modify the dataConfig.tpl and dataConsent.tpl files, in addition to the Psgdpr::getTokenFromAdminLink() method, to verify that the admin tokens were active.

@Codencode Codencode requested a review from nicohery May 5, 2026 07:23
Comment thread views/templates/admin/tabs/dataConfig.tpl Outdated
Comment thread views/templates/admin/tabs/dataConsent.tpl Outdated
@Codencode Codencode force-pushed the fix-241--Wrong-POST-action-throws-error-in-PS-9.1 branch from deae3c1 to 2d69163 Compare May 5, 2026 07:29
@Codencode Codencode requested review from Hlavtox and clotairer May 5, 2026 07:30
@Codencode

Copy link
Copy Markdown
Contributor Author

AI suggested that duplicate parameters can occur. :-)
What about this?

function setQueryParameter(url, key, value) {
  const u = new URL(url, window.location.origin);
  u.searchParams.set(key, value);
  return u.toString();
}

window.history.pushState({}, "", setQueryParameter(moduleAdminLink, "page", item));

Ok, I made the change. I also created an additional commit, because I realized that we also needed to modify the dataConfig.tpl and dataConsent.tpl files, in addition to the Psgdpr::getTokenFromAdminLink() method, to verify that the admin tokens were active.

ping @Hlavtox

Comment thread views/templates/admin/tabs/dataConsent.tpl Outdated
Comment thread psgdpr.php Outdated
@Codencode

Copy link
Copy Markdown
Contributor Author

@Hlavtox, you're actually right, the links can be retrieved directly by passing the 'page' parameter. I hadn't considered it because I didn't want to change the current logic. I've made the changes — do you think it's okay now

@Codencode Codencode requested a review from Hlavtox May 13, 2026 07:04

@Hlavtox Hlavtox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Paolo “Il Cleaner del Codice” Cunti 🚀

@ps-jarvis ps-jarvis added the Waiting for QA Status: Action required, Waiting for test feedback label May 13, 2026
@ps-jarvis ps-jarvis moved this from Ready for review to To be tested in PR Dashboard May 13, 2026
@Codencode Codencode added the Waiting for QA by Community Status: Action required, Waiting for test feedback by Community label May 26, 2026

@AureRita AureRita left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi @Codencode

Thank you for your PR, I tested it and it seems to works as you can see :

Capture.video.du.2026-05-29.10-41-42.mp4

Tested on :
9.1.x
9.0.3

The PR seems to works as expected, It'll be QA ✔️ when the PHP test 'll be green

Thank you

@Codencode

Copy link
Copy Markdown
Contributor Author

@Hlavtox @PrestaShop/committers
Can anyone tell me why there are errors on the CS-Fixer and PHPStan tests?

@jf-viguier

Copy link
Copy Markdown
Contributor

@Codencode please rebase, should be ✅

@Hlavtox Hlavtox closed this Jun 4, 2026
@github-project-automation github-project-automation Bot moved this from To be tested to Closed in PR Dashboard Jun 4, 2026
@Hlavtox Hlavtox reopened this Jun 4, 2026
@github-project-automation github-project-automation Bot moved this from Closed to Reopened in PR Dashboard Jun 4, 2026
@Codencode Codencode force-pushed the fix-241--Wrong-POST-action-throws-error-in-PS-9.1 branch from 6ab3f13 to 0c64cef Compare June 4, 2026 13:46
@Codencode Codencode force-pushed the fix-241--Wrong-POST-action-throws-error-in-PS-9.1 branch from 0c64cef to f8d53cd Compare June 4, 2026 14:01
@Codencode

Copy link
Copy Markdown
Contributor Author

@Codencode please rebase, should be ✅

Done!

Thank you @jf-viguier

@Codencode

Copy link
Copy Markdown
Contributor Author

Hi @Codencode

Thank you for your PR, I tested it and it seems to works as you can see :

Capture.video.du.2026-05-29.10-41-42.mp4
Tested on : 9.1.x 9.0.3

The PR seems to works as expected, It'll be QA ✔️ when the PHP test 'll be green

Thank you

@AureRita the tests are fine now.

@Hlavtox Hlavtox merged commit 3dfab15 into PrestaShop:dev Jun 4, 2026
12 checks passed
@ps-jarvis ps-jarvis moved this from Merged to Reopened in PR Dashboard Jun 4, 2026
@github-project-automation github-project-automation Bot moved this from Reopened to Merged in PR Dashboard Jun 4, 2026
@ps-jarvis

Copy link
Copy Markdown

PR merged, well done!

Message to @PrestaShop/committers: do not forget to milestone it before the merge.

@Hlavtox Hlavtox added QA ✔️ Status: Check done, Code approved and removed Waiting for QA by Community Status: Action required, Waiting for test feedback by Community Waiting for QA Status: Action required, Waiting for test feedback labels Jun 4, 2026
@Codencode Codencode deleted the fix-241--Wrong-POST-action-throws-error-in-PS-9.1 branch June 5, 2026 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA ✔️ Status: Check done, Code approved

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Wrong POST action throws error in PS 9.1

7 participants