system test fix (with revert)

[jlvallelonga]

fixes system test failures from before #576, but reverts #576 since more tests were failing with that and I'm not sure how to fix them yet.

[mattlindsey]

I'm glad that the Webmock.disable fixed the earlier problem, but now it looks like something with the scrolling. Maybe adding the <turbo-frame> to the navbar in PR 576 messed up the scrolling since taking that out fixed the tests? There's a nav-scrollable class that controls it.

Do you see what the actual problem is? The scrolling appears to work fine when I run the app.

(Note that #581 fixes a refresh problem, but NOT the scrolling tests.)

[jlvallelonga]

I'm glad that the Webmock.disable fixed the earlier problem, but now it looks like something with the scrolling. Maybe adding the <turbo-frame> to the navbar in PR 576 messed up the scrolling since taking that out fixed the tests? There's a nav-scrollable class that controls it.

Do you see what the actual problem is? The scrolling appears to work fine when I run the app.

(Note that #581 fixes a refresh problem, but NOT the scrolling tests.)

Yeah glad we have something that gets the tests to pass despite having to revert the changes from #576. Maybe we can go ahead with this and then add another PR for the change from #576 that also has passing tests. That would let us merge some of the pending PRs at least. What do you think @krschacht? Definitely was hard to see if anything new was breaking tests since some of the other problems were causing everything to fail.

[jlvallelonga]

on the test/system/conversations/messages/new_conversation_test.rb:17 test specifically, it's failing because the conversation is not selected. The screenshot is from running the test on this branch with a binding.pry in that test after line 15 to debug. When I run the test on main, the conversation isn't selected so that test is failing.

[mattlindsey]

@jlvallelonga I tracked down what code in #576 is causing the tests to fail.
If you comment out the code in app/javascript/stimulus/search_controller.js that automatically submits the search after 900 ms (line 11 and lines 24-27) all of the system tests pass (for me at least). The system tests do sleep in several places, which apparently conflicts with the auto submit.
I think you could disable those lines instead of reverting if you want. Whatever you guys decide is fine.

[mattlindsey]

@jlvallelonga Sorry. I think I was wrong about that. The timer may be an issue, but the biggest problem still seems to be the scroll position getting changed very slightly (in addition to the issue you found above). I can't figure it out.

[krschacht]

Thanks for fixing that core failing test, @jlvallelonga. I was a bit hesitant to merge this in and revert the conversation search feature, so I dug into the failing tests. I believe there were 2 legit failing tests and I fixed them both. I'll merge this in now.

@mattlindsey Let me know if you see any issues in main now since I did make some minor modifications to the conversation search feature. I went ahead and merged this in since I think it's an unambiguous step forward, but we can keep fixing forward if we discover additional issues.