Release 1.165.0 - #3915
Closed
odlbot wants to merge 8 commits into
Closed
Release 1.165.0#3915odlbot wants to merge 8 commits into
odlbot wants to merge 8 commits into
Conversation
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…e Keycloak dataclasses (#3908) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eateBasketWithProductsSerializer (#3902) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
OpenAPI ChangesShow/hide changesUnexpected changes? Ensure your branch is up-to-date with |
Comment on lines
+718
to
+722
| if bool(attrs.get("codes")) == bool(attrs.get("prefix")): | ||
| msg = "Supply either codes, or a prefix to generate them from." | ||
| raise serializers.ValidationError({"codes": msg}) | ||
|
|
||
| if attrs.get("codes") and "count" in attrs: |
Contributor
There was a problem hiding this comment.
Bug: The validation error message in BulkDiscountSerializer is ambiguous when both codes and prefix are supplied, as it doesn't state they are mutually exclusive.
Severity: LOW
Suggested Fix
Update the validation logic to provide a more specific error message when both codes and prefix are present. For example, check if both are truthy and raise a ValidationError with a message like "'codes' and 'prefix' are mutually exclusive; supply one or the other, but not both."
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: ecommerce/serializers/__init__.py#L718-L722
Potential issue: The validation logic in `BulkDiscountSerializer` raises the same error
message for two distinct invalid states: when neither `codes` nor `prefix` is provided,
and when both are provided. While functionally correct in rejecting invalid input, the
message "Supply either codes, or a prefix to generate them from" is ambiguous and
misleading for the case where both are provided, as it doesn't clarify that they are
mutually exclusive. This is a user experience issue that could cause confusion for API
users trying to debug their requests.
Did we get this right? 👍 / 👎 to inform future reviews.
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
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.
annagav
Chris Chudzicki
cp-at-mit
Asad Ali
Nathan Levesque