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

Support File Uploads as Nested Properties Within Multi Typed Object #2078

Merged
merged 3 commits into from Nov 30, 2020
Merged

Support File Uploads as Nested Properties Within Multi Typed Object #2078

merged 3 commits into from Nov 30, 2020

Conversation

iantasker
Copy link
Contributor

1️⃣ Is this something that is wanted/needed? Did you create an issue / discussion about it first?

This is a bug fix for #1714

2️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out.

No

3️⃣ Does it include tests, if possible? (Not a deal-breaker, just a nice-to-have)

Yes

4️⃣ Please include a thorough description of the improvement and reasons why it's useful.

Its resolves an an issue with the file upload component not correctly rehydrating an object with non string or array properties after a file has been uploaded.

5️⃣ Thanks for contributing! 🙌

@iksaku
Copy link
Contributor

iksaku commented Nov 22, 2020

Super clean fix 🤩

How about changing reference from "numbers can be updated"? It is only a side-effect of losing track of the state of the property, so I suggest renaming to something referencing "Support File Uploads for Nested Component Properties".

@iantasker
Copy link
Contributor Author

@iksaku It might be easier to extend the it_can_upload_single_file_with_in_array_public_property and it_can_upload_multiple_file_with_in_array_public_property tests as the scopes are pretty much the same.

Would you be happy with that?

@iksaku
Copy link
Contributor

iksaku commented Nov 22, 2020

Sure, that would better describe what the test is intended to do.

PS: I think it is written "within" (without space) 🤔.

@iantasker
Copy link
Contributor Author

@iksaku I will also update the test name as I agree it should be within not with in

@iantasker iantasker changed the title File number hydrate issue Support File Uploads as Nested Properties Within Multi Typed Object Nov 22, 2020
@stancl
Copy link
Contributor

stancl commented Nov 22, 2020

Just tested this in Lean and it works perfectly.

Screen Shot 2020-11-22 at 20 18 22

Screen Shot 2020-11-22 at 20 20 04

Screen Shot 2020-11-22 at 20 17 03

As I said in #1714, I'll be paying a $100 bounty for this.

@iantasker please DM me your payment details on Discord. I'll also invite you to Lean for free.

@calebporzio please merge this soon, it's tested both with PHPUnit and in a real app and I'd really love to be able to use this 😅

@iantasker
Copy link
Contributor Author

@iksaku I have consolidated the tests so this is good to merge now.

@calebporzio
Copy link
Collaborator

Fantastic work everyone. Thanks

@calebporzio calebporzio merged commit d3b66aa into livewire:master Nov 30, 2020
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.

None yet

5 participants