germsvel
Error when testing add/remove of nested forms - data getting out of sync?
This is a continuation of #170 , with a sample app for reproduction!
I suspect I maybe be holding the tool slightly wrong, but the behaviour with PhoenixTest is definitely different than what I see when using the demo app in a browser.
To reproduce:
- Clone https://github.com/sevenseacat/phoenix_test_remove_bug
-
mix deps.get - Run
mix test test/posts_live_test.exs
This test file exercises the PostsLive liveview, which has a form for a post with some nested comments. The test adds three comments and populates content for them, removes the second one, submits the form, and prints out the received params.
In a browser I correctly see that the submitted params were for comments AAA and CCC (the first and third comments)
However, the test prints out that the comments AAA and BBB were submitted (despite showing form inputs for comments AAA and CCC).
@sevenseacat thanks so much for adding a demo app that reproduces the error! 🙏 Can't tell you how helpful that is.
I've spent a few hours looking at this, and I see what's happening (but don't know how to fix it yet).
The problem
Your form uses index based positions -- so you had "AAA" as index 0, "BBB" as index 1, and "CCC" as index 2.
When you remove "BBB", "CCC" becomes index 1.
The problem is that PhoenixTest knows nothing about the removal. All we can see is the HTML on the page and what you've "filled in" across time.
So, PhoenixTest knows you filled in index 0 with "AAA", 1 with "BBB", and 2 with "CCC".
When you submit the form, we look to see which fields are still present on the page and remove things that no longer exist on the page. But since the page has fields with index 0 and 1, PhoenixTest removes index 2 with "CCC" -- since that's what disappeared from the page.
I'll have to think if there's a way to make the removal of non-existent data smarter.
I'm going to ping Zach (from Ash core team) about this and see if we need to make AshPhoenix smarter - if it removed index 1 from the data completely, and left everything alone at indexes 0 and 2, that would work better?
I'm not sure how Ecto would handle this scenario, so I can't really compare it unfortunately
Yeah, I don't know that Ash is doing anything out of the ordinary there. I think it's just a limitation of how much PhoenixTest knows about the changing UI, and how we keep track of what you fill in.
@sevenseacat I think I have a solution for your problem. Just got merged in #208. Can you test out main whenever you have a chance and see if that fixes your issues. And thanks again for the app to reproduce the error. It was sooo helpful.
@sevenseacat I'm sad to say that I tested out main on a couple of apps that I use to check for regressions, and the changes in #208 introduce a bad regression for LiveViews that don't fully control their inputs. So... I'm going to have to revert the work on #208 and go back to the drawing board on this.
I think we need to support this somehow, but I don't exactly know how.
Ah poop! And that would definitely be a more common use case than this. All good!