这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@JohnVillalovos
Copy link
Contributor

…ources

When updating a resource via the API, existing custom attribute values were not being removed before adding new ones, causing duplicate entries in the custom_attribute_values table.

The issue occurred because BuildResource() created a fresh BookableResource object with empty attributes, so ChangeAttributes() couldn't detect which attributes should be removed.

  • Add GetAttributeValues() method to BookableResource with copy protection
  • Copy existing attributes to new resource before calling ChangeAttributes()

Copilot AI review requested due to automatic review settings July 13, 2025 15:55

This comment was marked as outdated.

@JohnVillalovos JohnVillalovos force-pushed the jlvillal/attributes_2 branch from 5bd9979 to c5f875c Compare July 20, 2025 17:26
@JohnVillalovos JohnVillalovos requested a review from Copilot July 20, 2025 17:28
Copy link

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

This PR fixes a bug where updating resources via the API was creating duplicate custom attribute values instead of properly replacing them. The core issue was that BuildResource() created a fresh BookableResource object without existing attributes, preventing ChangeAttributes() from detecting which attributes should be removed.

  • Add GetAttributeValues() method to BookableResource with copy protection using spread operator
  • Modify BuildResource() to accept an optional existingResource parameter and copy existing attributes before calling ChangeAttributes()
  • Update both Create() and Update() methods to use named parameters for better readability

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
WebServices/Controllers/ResourceSaveController.php Modified BuildResource() method to accept existing resource and copy attributes; updated method calls to use named parameters
Domain/BookableResource.php Added GetAttributeValues() method that returns a copy of attribute values array

* @return BookableResource
*/
private function BuildResource($request, $resourceId)
private function BuildResource(ResourceRequest $request, int $resourceId, ?BookableResource $existingResource = null): BookableResource
Copy link

Copilot AI Jul 20, 2025

Choose a reason for hiding this comment

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

The method signature change introduces inconsistency between Create and Update calls. In Create, existingResource will always be null, making the parameter unnecessary. Consider creating a separate method like BuildResourceFromExisting() for the Update case to maintain clearer API semantics.

Copilot uses AI. Check for mistakes.
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/attributes_2 branch 2 times, most recently from 4c59467 to 76d5145 Compare July 22, 2025 15:54
…ources

When updating a resource via the API, existing custom attribute values
were not being removed before adding new ones, causing duplicate entries
in the custom_attribute_values table.

The issue occurred because BuildResource() created a fresh BookableResource
object with empty attributes, so ChangeAttributes() couldn't detect which
attributes should be removed.

  * Add GetAttributeValues() method to BookableResource with copy
    protection
  * Copy existing attributes to new resource before calling
    ChangeAttributes()
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/attributes_2 branch from 76d5145 to ad3a01b Compare July 22, 2025 18:10
@JohnVillalovos
Copy link
Contributor Author

I did some testing and this did not work ☹️ Will go with other approach in #681

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