Skip to content

WIP - Refactor custom fields handling for storage#15044

Open
snipe wants to merge 1 commit intodevelopfrom
refactor_custom_fields_saving
Open

WIP - Refactor custom fields handling for storage#15044
snipe wants to merge 1 commit intodevelopfrom
refactor_custom_fields_saving

Conversation

@snipe
Copy link
Member

@snipe snipe commented Jul 8, 2024

This just condenses down the custom field stuff into a smaller method. @uberbrady - I know you were working on something similar for the upcoming custom fields for users, but this should be a nicer way for us to bridge that gap. I just need to make sure the additional tests are there. We do something slightly different with bulk updating assets, since we're passing it an updated array there, so I didn't touch that for now.

Signed-off-by: snipe <snipe@snipe.net>
@what-the-diff
Copy link

what-the-diff bot commented Jul 8, 2024

PR Summary

  • Implementation of New Method for Custom Fields
    We have introduced the handleCustomFieldsForStoring method in the Asset model. This new method is designated to manage updates to the custom fields.

  • Consolidating Code in AssetsController
    We have streamlined the code in both Api/AssetsController.php and AssetsController.php files. The sections dealing with the update of custom fields in store and update methods have been replaced with a call to the new handleCustomFieldsForStoring method. This results in cleaner and more efficient code.

  • Inclusion of Additional Facades in Asset model
    We have included usage of Illuminate\Support\Facades\Crypt and Illuminate\Support\Facades\Gate in the Asset model. They provide important functionalities necessary for encryption processes and authorization checks.

@snipe
Copy link
Member Author

snipe commented Jul 8, 2024

Tests failing here (only on Sqlite) - looking into it

@snipe snipe requested a review from uberbrady July 11, 2024 08:12
@snipe
Copy link
Member Author

snipe commented Mar 6, 2026

@uberbrady Is there anything here worth salvaging, or should I just close it?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant