Skip to content

Add a conditional alert message for TOS validation#892

Merged
Hlavtox merged 3 commits intoPrestaShop:developfrom
tblivet:feat/tos-cms-conditions
Jan 9, 2026
Merged

Add a conditional alert message for TOS validation#892
Hlavtox merged 3 commits intoPrestaShop:developfrom
tblivet:feat/tos-cms-conditions

Conversation

@tblivet
Copy link
Contributor

@tblivet tblivet commented Jan 9, 2026

Questions Answers
Description? Add a conditional alert message for TOS validation
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? #887
Sponsor company @PrestaShopCorp
How to test? Follow this issue: #887

@tblivet tblivet requested a review from ga-devfront January 9, 2026 08:42
@github-project-automation github-project-automation bot moved this to Ready for review in PR Dashboard Jan 9, 2026
@tblivet tblivet linked an issue Jan 9, 2026 that may be closed by this pull request
2 tasks
nicosomb
nicosomb previously approved these changes Jan 9, 2026
@ps-jarvis ps-jarvis moved this from Ready for review to To be tested in PR Dashboard Jan 9, 2026
ga-devfront
ga-devfront previously approved these changes Jan 9, 2026
@ingridusta ingridusta self-assigned this Jan 9, 2026
ingridusta
ingridusta previously approved these changes Jan 9, 2026
Copy link

@ingridusta ingridusta left a comment

Choose a reason for hiding this comment

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

Hi @tblivet,

Your fix works perfectly ✨
Alert message is now consistent with the configuration in the BO + no regression on "Page for terms and conditions" configuration :

Enregistrement.de.l.ecran.2026-01-09.a.11.25.12.mov
approved-qa-nana

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

@tblivet Please, check my comment on the issue. It probably won't work if bultin TOS are disabled, but other checkboxes are present.

If you use !empty($conditions_to_approve), it will work. :-)

@tblivet tblivet dismissed stale reviews from ingridusta, ga-devfront, and nicosomb via bb9b110 January 9, 2026 10:58
nicosomb
nicosomb previously approved these changes Jan 9, 2026
@tblivet
Copy link
Contributor Author

tblivet commented Jan 9, 2026

Hi @Hlavtox, I’m not sure if I fully understand, but I’ve pushed a commit that fixes what I understood ^^
This should take into account the case you described, can you confirm ?

@Hlavtox
Copy link
Contributor

Hlavtox commented Jan 9, 2026

I will make a simple test module, 5 mins

@Hlavtox
Copy link
Contributor

Hlavtox commented Jan 9, 2026

@tblivet @ingridusta
tox_tos.zip

Install this, it will add another required checkbox to checkout conditions. :-)

Snímek obrazovky 2026-01-09 122141

Try all 4 cases. From looking at the current code, it should work perfectly. :-)

Presta default terms and conditions Other conditions from module Result?
No No Please make sure you've chosen a [1]payment method[/1]
Yes No Please make sure you've chosen a [1]payment method[/1] and accepted the [2]terms and conditions[/2].
No Yes Please make sure you've chosen a [1]payment method[/1] and accepted the required condition(s).
Yes Yes Please make sure you've chosen a [1]payment method[/1] and accepted the required condition(s).

@tblivet
Copy link
Contributor Author

tblivet commented Jan 9, 2026

@Hlavtox thank you 🙏 We will check with your module !

@tblivet
Copy link
Contributor Author

tblivet commented Jan 9, 2026

Thanks @Hlavtox, all cases work well. I’ve added a small style improvement. I’ll let @ingridusta retest then 🙂

Copy link

@ingridusta ingridusta left a comment

Choose a reason for hiding this comment

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

Hi @tblivet & @Hlavtox,
I retested all the cases submitted by @Hlavtox.
It works as expected.

Case 1 :
Case 1

Case 2 :
Case 2

Case 3 :
Case 3

Case 4 :
Case 4

It's QA approved ✅

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

Thank you both! :-) Now we have it universal!!

@Hlavtox Hlavtox merged commit ee8d0e0 into PrestaShop:develop Jan 9, 2026
6 checks passed
@github-project-automation github-project-automation bot moved this from To be tested to Merged in PR Dashboard Jan 9, 2026
@ps-jarvis
Copy link

PR merged, well done!

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

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

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[BUG] Last alert message before checkout is not updated according to TOS status

6 participants