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

Updating model with a json-casted field and guarded id does not work with terse arrow syntax #40506

Closed
fylzero opened this issue Jan 20, 2022 · 6 comments

Comments

@fylzero
Copy link
Contributor

fylzero commented Jan 20, 2022

  • Laravel Version: 8.80.0
  • PHP Version: 8.1.1
  • Database Driver & Version: MySQL Ver 8.0.27 for macos12.0 on arm64 (Homebrew)

Description:

When attempting to update a JSON-casted field (not the id field) using the terse/arrow syntax on a model that has $guarded = ['id'] the intended json field does not change/update.

Steps To Reproduce:

  • Create a migration and model that contains a JSON field
  • Add $guarded = ['id'] and protected $casts = ['json_field' => 'json']; to the model
  • Use the terse syntax to update part of the json field data - https://laravel.com/docs/8.x/eloquent-mutators#array-and-json-casting (bottom of section). Example: $user->update(['options->key' => 'value']);
  • The field does not update

Not sure why this is happening specifically when the id field is guarded even when you are not explicitly updating the id column. If you remove that guard, it will update as expected.

https://github.com/fylzero/bug-report - Follow the readme file, view the model and tests

@driesvints
Copy link
Member

Hey @fylzero. This is intentional. We don't support mass updating of those attributes anymore because it's a security risk. It seems we forgot to remove that section in the docs. I'll get it removed.

See #33777

@fylzero
Copy link
Contributor Author

fylzero commented Jan 20, 2022

@driesvints Makes sense. Thanks for looking into this. I was following advice from here: https://www.youtube.com/watch?v=wPOCQUDUsVc

Planning to move back to $fillable per Taylor's recommendations here: https://blog.laravel.com/security-release-laravel-61835-7240

@driesvints
Copy link
Member

Hey. @fylzero i indeed was wrong. Needs to be fillable: laravel/docs#7606 (comment)

@fylzero
Copy link
Contributor Author

fylzero commented Jan 20, 2022

@driesvints I actually figured that out just before you responded, but thank you. One thing that might be worth noting is that you now need to apparently place the arrow syntax reference in your fillable property like so: $fillable = ['json_db_field->json_key'] for this to work. That should be added to the docs for sure. Let me know if you agree / want to add that or would like me to submit a PR to the docs. Thanks!

Edit: Oop! Nevermind, I see where Taylor pointed to this in the docs. Might be good to link that as a reference here: https://laravel.com/docs/8.x/eloquent-mutators#array-and-json-casting

That would seem useful as a note there.

@ludgerey
Copy link

Thanks for figuring that out. It should definitely go in the docs, as there is already a example, that simple doesn't work. I assumed that I had to add my json field name, but not that I had to add the keys too.

@fylzero
Copy link
Contributor Author

fylzero commented Dec 6, 2023

Thanks for figuring that out. It should definitely go in the docs, as there is already a example, that simple doesn't work. I assumed that I had to add my json field name, but not that I had to add the keys too.

Submitted a PR

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

No branches or pull requests

3 participants