Skip to content

Fix psgdpr menu URL query separator#242

Open
Codencode wants to merge 4 commits into
PrestaShop:devfrom
Codencode:fix-241--Wrong-POST-action-throws-error-in-PS-9.1
Open

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

Conversation

@Codencode
Copy link
Copy Markdown

@Codencode Codencode commented Apr 23, 2026

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
Copy link
Copy Markdown

Hello @Codencode!

This is your first pull request on psgdpr repository of the PrestaShop project.

Thank you, and welcome to this Open Source community!

@clotairer
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

@Hlavtox Hlavtox left a comment

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
Copy link
Copy Markdown

ps-jarvis commented May 5, 2026

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
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
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
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
Copy link
Copy Markdown
Contributor

@Hlavtox Hlavtox left a comment

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
Copy link
Copy Markdown

@AureRita AureRita left a comment

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
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting for author Waiting for QA by Community Status: Action required, Waiting for test feedback by Community Waiting for QA Status: Action required, Waiting for test feedback

Projects

Status: To be tested

Development

Successfully merging this pull request may close these issues.

Wrong POST action throws error in PS 9.1

6 participants