Skip to content

Conversation

@CySSoO
Copy link
Contributor

@CySSoO CySSoO commented Oct 7, 2025

Summary

  • replace pretty blocks field definitions using the number type with text to allow non-numeric input

Testing

  • not run (not requested)

https://chatgpt.com/codex/tasks/task_e_68e4df48d6b483229d33725251978584

@CySSoO CySSoO merged commit 2547e67 into master Oct 7, 2025
0 of 6 checks passed
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

Comment on lines 3170 to 3173
'max_winners' => [
'type' => 'number',
'type' => 'text',
'label' => $module->l('Maximum winners for this segment (0 for unlimited)'),
'default' => '0',

Choose a reason for hiding this comment

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

P1 Badge Allowing text input for winner limit bypasses numeric validation

The PrettyBlocks definition for max_winners now uses type => 'text', which means the admin UI no longer constrains the field to numbers. However the game controllers still cast this setting with (int) and treat 0 as “unlimited winners” (see controllers/front/wheel.php and controllers/front/scratch.php where max_winners is cast without validation). A non‑numeric input such as 'foo' will therefore become 0 and silently remove the intended cap on rewards. Either keep this field numeric or add server‑side validation to reject non‑numeric values before casting to avoid disabling winner limits by mistake.

Useful? React with 👍 / 👎.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants