Expose GetTaxRuleList as GET /tax-rules-groups/{id}/tax-rules - #1
Closed
PrestaEdit wants to merge 360 commits into
Closed
PrestaEdit wants to merge 360 commits into
PrestaEdit wants to merge 360 commits into
Conversation
…mprove doc for bulk delete
…te import for bulk-update-status
…meter (422 instead of 400)
…y needs to send an empty JSON object to test
…elete-discount Bulk enable disable delete discount
…quantity-and-quantity-per-user Make nullable discount quantity and quantity per user
Updated contributing section to include API contributions and resources.
Updated README for clarity and grammar improvements.
Co-authored-by: Jonathan Lelievre <jo.lelievre@gmail.com>
… test - Rename $localizedNames/$localizedPublicNames to $names/$publicNames and bridge via ApiResourceMapping (matches DiscountTypeList precedent). - Integration test: type-assert every field, assert nested attributes[] item shape, and verify the default Color group (id=2) exposes the expected aggregate: name 'Color', groupType 'color', colorGroup true, 14 attributes each with a non-null color hex. - Document the $attributes invariant: DTO getter returns null when attributes weren't queried, but this endpoint always queries them. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ning - Mark $attributeGroupId as ApiProperty identifier - List all fields explicitly in ApiResourceMapping (follow DiscountTypeList precedent) - Rename resource field groupType -> type to match AttributeGroup.php - Document intentional shopIds omission (GetAttributeGroupList DTO does not return shop associations) - Test: drop redundant createApiClient, assert en-US and fr-FR locale keys explicitly on names/publicNames, add getItem-on-collection comment Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The nested items in $attributes come straight from the Core DTO Attribute\QueryResult\Attribute and serialize as attributeId, position, color, localizedNames, textureFilePath — not name/imagePath. Align the test assertions and the openapiContext schema with the actual response. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Remove tracked .DS_Store and gitignore it - Rewrite AttributeGroupWithAttributesEndpointTest to a single assertEquals on the full default-fixture aggregate (per @jolelievre's review), so any new field on the group or attribute row surfaces as a test failure Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace hardcoded snapshot of demo attribute ids, colors, and fr-FR translations with structural assertions: keys, types, and the colorGroup <-> type=='color' invariant. Also allow textureFilePath to be null. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Belongs in the user's global gitignore per review feedback. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses @jolelievre's CHANGES_REQUESTED on PR PrestaShop#390: 1. Restore assertEquals-on-full-aggregate test The previous assertArrayHasKey-per-field variant was too permissive — a new field or a wrong value would slip through. The full snapshot forces any change on the group / attribute row to surface as a diff. 2. Fix nested localizedNames to be indexed by locale, not id_lang CQRSApiSerializer::normalizeLocalizedValues() rewrites id_lang → locale only for top-level #[LocalizedValue] properties on the API resource. The per-attribute `localizedNames` nested inside AttributeGroupWithAttributes::$attributes was therefore leaking language IDs into the public API, breaking the locale-only contract of every other localized field. AttributeGroupWithAttributesNormalizer is a narrow post-processing normalizer that delegates to the standard pipeline and then rewrites only that one sub-array via LangRepository::getMapping(). Whitelisted in ApiResourceNormalizerRule with justification, wired in config/admin/services.yml. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two CI failures from the previous push: 1. Integration tests: the demo-fixture snapshot could not match because attribute IDs, colors and even group types drift between core versions (9.0.3 / 9.1.x / 9.2.x / develop) and the fr-FR translations don't exist in the default dump. Rewritten the test to create its own attribute group + two attributes via POST (both localized in en-US / fr-FR), then assertEquals on the full aggregate row for that group. Same "full structure" guarantee @jolelievre asked for, but version-independent. 2. PHP-CS-Fixer: dropped the aligned @PARAM padding. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…e and rename attribute sub field to names instead localizedNames
…s-with-attributes-endpoint Add GetAttributeGroupList Admin API endpoint (rich aggregate)
fix (cherry picked from commit 9c4a3c5)
…rs-in-summary-command Fix handling open prs in summary command + more condensed report
…because not relevant
Restore the paginated fetchGitHubPRs implementation from fb49c86, which was lost when PR PrestaShop#405 was merged into dev. GitHub caps per_page at 100, so the single-request version silently truncated the open PR list.
…ts-tracking-table Add possibility to ignore commands / queries + handling more than 50PRs
…9.1.4 Replace 9.1.x branch with 9.1.4 tag in CI
Bumps [squizlabs/php_codesniffer](https://github.com/PHPCSStandards/PHP_CodeSniffer) from 3.7.2 to 3.13.6. - [Release notes](https://github.com/PHPCSStandards/PHP_CodeSniffer/releases) - [Changelog](https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/4.x/CHANGELOG-3.x.md) - [Commits](PHPCSStandards/PHP_CodeSniffer@3.7.2...3.13.6) --- updated-dependencies: - dependency-name: squizlabs/php_codesniffer dependency-version: 3.13.6 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
…/squizlabs/php_codesniffer-3.13.6 Bump squizlabs/php_codesniffer from 3.7.2 to 3.13.6
Adds a paginated sub-resource returning the tax rules attached to a
tax rules group. The endpoint delegates to the existing core CQRS query
GetTaxRuleList (taxRulesGroupId + languageId + optional limit/offset).
Fields exposed match the TaxRuleForList DTO returned by the core handler:
taxRuleId, countryName, stateName, zipcode, behavior, taxName, taxRate,
description — plus the parent taxRulesGroupId injected from the URI.
Scope: tax_rules_group_read (mirrors /products/{id}/images convention).
Tracks PrestaShop/PrestaShop#39630.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The GetTaxRuleList CQRS query only exists on PrestaShop develop. Mark the operation experimental, guard the tests with class_exists(), and add PHPStan version ignores for 9.0.3 and 9.1.4 — same treatment as the TaxRule Command/Exception classes already handled. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ApiPlatform interprets #[ApiProperty(identifier: true)] on a collection
sub-resource as the URL variable target, which shadowed the natural
{taxRulesGroupId} URI variable and produced 'Invalid identifier value
or configuration' 404s. ProductImageList / CombinationList follow the
same convention with no identifier annotation, so align on that.
Also pin requirements: ['taxRulesGroupId' => '\d+'] for consistency
with the main TaxRulesGroup resource operations.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
testProtectedEndpoints expected a 401 but got 404 on 9.0/9.1 because the experimental route is not registered when the CQRS class is absent. Guard the yield with class_exists() to mirror the runtime gating. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 of 3 tasks
Owner
Author
|
CI validated on the full matrix (see final commit dab0125). Superseded by upstream PR: PrestaShop#411 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a paginated sub-resource returning the tax rules attached to a tax rules group, delegating to the existing core CQRS query
GetTaxRuleList.GET /tax-rules-groups/{taxRulesGroupId}/tax-rules?limit=&offset=PrestaShop\PrestaShop\Core\Domain\TaxRulesGroup\TaxRule\Query\GetTaxRuleListTaxRuleList { taxRules[], totalCount }→ mapped viaitemsField/countFieldlanguageIdsourced from request context ([_context][langId]), same pattern asCombinationListtax_rules_group_read(mirrorsproduct_readfor/products/{id}/images)Fields exposed match
TaxRuleForList:taxRuleId,countryName,stateName,zipcode,behavior,taxName,taxRate,description, plustaxRulesGroupIdfrom the URI.Tracks upstream tracking issue: PrestaShop/PrestaShop#39630
Test plan
composer setup-local-tests+composer run-module-tests --filter TaxRulesGroupEndpointTest/tax-rules-groups/1/tax-ruleswith a valid access token, confirm response shape and paginationDraft — opening on the fork first to let CI validate before retargeting
PrestaShop/ps_apiresources:dev.🤖 Generated with Claude Code