Skip to content

(feat): add percentage field to Leges#15

Open
SimonvanWijhe wants to merge 9 commits intomasterfrom
feat/percentage
Open

(feat): add percentage field to Leges#15
SimonvanWijhe wants to merge 9 commits intomasterfrom
feat/percentage

Conversation

@SimonvanWijhe
Copy link
Copy Markdown
Contributor

@SimonvanWijhe SimonvanWijhe commented Mar 13, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for storing and displaying a “percentage” value on Leges across the metabox, admin list/quick edit, shortcode rendering, and REST output.

Changes:

  • Adds _pdc-lege-percentage to Leges meta and exposes it via REST (percentage) and admin list column.
  • Updates shortcode rendering to output a percentage when set, and updates unit tests accordingly.
  • Extends quick edit to include the percentage field and introduces 4-decimal sanitization in NumberSanitizer.
  • Updates composer.lock with broad dependency upgrades.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/Unit/Leges/Shortcode/ShortcodeTest.php Adds unit test coverage for percentage shortcode output.
tests/Unit/Leges/Admin/QuickEdit/QuickEditServiceProviderTest.php Updates quick edit row-actions tests for the new percentage meta attribute.
src/Leges/Traits/NumberSanitizer.php Adds 4-decimal float sanitization via a shared helper.
src/Leges/Shortcode/Shortcode.php Adds percentage meta extraction + early return rendering path.
src/Leges/RestAPI/Repositories/LegesRepository.php Adds percentage field to REST transform output.
src/Leges/PostType/LegesPostTypeServiceProvider.php Adds an admin column to display the percentage.
src/Leges/Metabox/Metabox.php Adds a metabox field for percentage with 4-decimal sanitization.
src/Leges/Admin/QuickEdit/QuickEditServiceProvider.php Adds percentage quick-edit handler + sanitize callback support; refactors field rendering.
composer.lock Large set of dependency upgrades (WordPress, Symfony, WP-CLI, etc.).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@mvdhoek1 mvdhoek1 left a comment

Choose a reason for hiding this comment

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

Moet er niet iets gedaan worden met het percentage? Bijvoorbeeld een procentuele verhoging doorvoeren ieder jaar? Of is het alleen voor display purposes only?

Er is een cron-job die de data checked van de leges waarop deze verhoogd moeten worden. Die vervangt de oude prijs met de nieuwe, vooraf ingestelde, prijs. Zou kunnen dat die nieuwe prijs bepaald moest worden o.b.v. het ingestelde percentage.

},
],
'percentage' => [
'title' => __('Lege percentage (%)', 'pdc-leges'),
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.

Ik weet even niet meer wat het doel is van de percentages, wellicht een kleine omschrijving erbij plaatsen?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ik heb de werking wat aangepast en een video toegevoegd aan deze PR, hopelijk is het daarmee wat duidelijker.

@SimonvanWijhe
Copy link
Copy Markdown
Contributor Author

Moet er niet iets gedaan worden met het percentage? Bijvoorbeeld een procentuele verhoging doorvoeren ieder jaar? Of is het alleen voor display purposes only?

Er is een cron-job die de data checked van de leges waarop deze verhoogd moeten worden. Die vervangt de oude prijs met de nieuwe, vooraf ingestelde, prijs. Zou kunnen dat die nieuwe prijs bepaald moest worden o.b.v. het ingestelde percentage.

Ik heb een veld voor een nieuw lege percentage toegevoegd en de cronjob aangepast. Het percentage kan gekozen worden in plaats van het prijs veld. Ter verduidelijking heb ik een toggle toegevoegd waarmee je kan wisselen tussen de prijs en percentage velden.

@SimonvanWijhe SimonvanWijhe requested a review from mvdhoek1 April 3, 2026 12:18
'title' => __('Lege percentage (%)', 'pdc-leges'),
'function' => function () {
$percentage = get_post_meta(get_the_ID(), "{$this->prefix}-percentage", true);
echo ('' !== $percentage && null !== $percentage) ? esc_html(str_replace('.', ',', $percentage)) . '%' : '';
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.

Ik zou hier alleen maar checken op datgeen wat je wilt, bijvoorbeeld:
is_string($percentage) && '' !== trim(percentage)

Stel dat er een keer een array voorbij komt dan heb je een fatal error.
Is wel een persoonlijke voorkeur, andere laten het liever crashen zodat de fout goed opvalt.

return $this->sanitizeFloatWithDecimals($value, 2);
}

/**
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.

Ik vind de since toevoeging aan docblocks wel handig:

/**
 * Sanitize and format a value to a float with four decimal places.
 *
 * @since NEXT
 */

{
$newPrice = get_post_meta($lege->ID, self::META_NEW_PRICE, true);

if (empty($newPrice)) {
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.

Als de nieuwe prijs 0.00 is blijft de oude prijs bestaan.

{
$newPercentage = get_post_meta($lege->ID, self::META_NEW_PERCENTAGE, true);

if (empty($newPercentage)) {
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.

Idem

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants