Skip to content

Conversation

matthewbennink
Copy link
Contributor

See #474.

@keithschacht
Copy link
Contributor

@matthewbennink any idea about that failing test? I re-ran it once just to make sure it wasn’t a flappy test, and it seems to be a legit failure.

@matthewbennink
Copy link
Contributor Author

@matthewbennink any idea about that failing test? I re-ran it once just to make sure it wasn’t a flappy test, and it seems to be a legit failure.

Well, I see the issue at least. It's important to only send the inner HTML to the client from the background job, which is what the Nokogiri logic was taking care of. I missed that the first time around because our own logic didn't depend on not sending the outer div. Thankfully your system tests caught it!

I'm trying to rework this to avoid Nokogiri and the best I've come up with thus far is to extract yet another partial so that we can just render the inner HTML content when needed. Does that seem like a reasonable solution to you?

(It's early and I'm not used to doing development within Docker, plus not being able to run the system tests locally has me doing CI-driven dev, but hopefully I'll have a good PR in place in the next week or so.)

@keithschacht
Copy link
Contributor

@matthewbennink now that you tracked down the issue to the ' substitution, we could just go back to nokogiri but tack on a replace to convert the single quote back to ' ?

@mattlindsey mattlindsey self-requested a review September 20, 2025 14:20
Copy link
Collaborator

@mattlindsey mattlindsey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment by Keith. Thanks!

@mattlindsey
Copy link
Collaborator

Closing this because it looks like it should be totally re-worked since app code has evolved.

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.

3 participants