Skip to content

Conversation

@mfarhan943
Copy link
Contributor

@mfarhan943 mfarhan943 commented Jul 10, 2025


Fix: Prevent Duplicate AttributeOptionGroup Creation

Resolves: #372

Context

The options field in AttributeOptionGroupSerializer is defined using:

slug_field="option"

This means the serializer works with data like:

[
  {"option": "Large"},
  {"option": "Medium"}
]

However, when updating existing instances, the model receives options as actual AttributeOption instances:

[<AttributeOption: Large>, <AttributeOption: Medium>, <AttributeOption: Small>, <AttributeOption: Rokeol>]

This mismatch caused find_existing_attribute_option_group to fail at detecting duplicates — resulting in unnecessary creation of duplicate option groups even when the name and options were the same.


What Changed

Updated find_existing_attribute_option_group to:

  • Accept and normalize both:

    • {"option": "Large"}-style dicts from the serializer, and
    • AttributeOption instances from the model.
  • Raise clear exceptions (ValueError, TypeError) for bad input.


  • Prevents duplicate option groups when posting or updating with the same name/options.
  • Supports both create and update flows.
  • Validated by new tests for error handling and matching behavior.

@mfarhan943
Copy link
Contributor Author

@samar-hassan Kindly review.

self.response = self.put(
"admin-product-list",
**{"product_class": "t-shirt", "slug": "oscar-t-shirt-henk"}
**{"product_class": "t-shirt", "slug": "oscar-t-shirt-henk"},
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove these trailing commas if its not new code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

please add it again, lint fails

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@mfarhan943 mfarhan943 force-pushed the fix-atribute-duplicate branch 2 times, most recently from 1a73353 to 96f733e Compare July 15, 2025 11:13
@samar-hassan
Copy link
Contributor

please rebase your branch

@mfarhan943 mfarhan943 force-pushed the fix-atribute-duplicate branch 2 times, most recently from 485b507 to cb1b848 Compare July 15, 2025 15:28
@mfarhan943 mfarhan943 force-pushed the fix-atribute-duplicate branch from cb1b848 to 60449de Compare July 15, 2025 15:34
@samar-hassan samar-hassan merged commit c9a930b into django-oscar:main Jul 15, 2025
21 checks passed
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.

2 participants