Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quick revert for #13830 #13923

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Quick revert for #13830 #13923

merged 1 commit into from
Nov 22, 2023

Conversation

snipe
Copy link
Owner

@snipe snipe commented Nov 22, 2023

This PR partially reverts #13830, which created a regression where auto-incrementing broke for API requests where an asset tag was not present. The logic of putting that into a form request is sound, but I think it has too many echo effects right now. This PR removes the reference to the StoreAssetRequest and reverts it to the ImageUploadRequest, but does not actually delete the StoreAssetRequest. We might be better suited to use mutators or at least rethink the form request rules.

Copy link

@snipe snipe merged commit eae98d3 into develop Nov 22, 2023
4 of 7 checks passed
@snipe snipe deleted the bug/sc-24098 branch November 22, 2023 12:28
Copy link

what-the-diff bot commented Nov 22, 2023

PR Summary

  • Modified AssetsController store method parameters
    The store method in the AssetsController now takes an ImageUploadRequest instead of a StoreAssetRequest. This change might enhance the specificity and validation for image uploads in asset creation.

  • Added user permission check
    A new check has been introduced to ensure that only authorized users can create a new asset. This addition improves the security and restricts misuse of the application.

  • Removed assignment of model_id to Asset
    The direct assignment of model_id to Asset has been removed. This might help avoid wrong assignments and improve data integrity.

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