Lets use this as a @laravelphp learning opportunity.
We added a feature to let @thankbox recipients reply to everyone who had left a message on their Thankbox.
Lets look at the code and see what happened 1/7https://twitter.com/ValCanBuild/status/1273545648219062272 …
-
Pokaż ten wątek
-
This is the code that ran (shortened for brevity) when a recipient replied. The comment makes it pretty clear what we *wanted* it to do. Pretty trivial, right? Get all the messages on this Thankbox that have an email attached or that have a user on them. 2/7pic.twitter.com/MVGYcVvpPB
1 odpowiedź 0 podanych dalej 0 polubionychPokaż ten wątek -
Why do we need that orWhereHas? It's because we don't require people to register in order leave a message on a Thankbox - for ease of use. They can just leave their email (contributor_email). We don't ask users for their email if they are logged in - we know it. 3/7
1 odpowiedź 0 podanych dalej 0 polubionychPokaż ten wątek -
But we naively assumed that the orWhereHas would apply *just* to messages within the Thankbox we were querying. We were fooled by our unit tests, because in our tests we just created one thankbox and a few users so all assertions passed. 4/7
1 odpowiedź 0 podanych dalej 0 polubionychPokaż ten wątek -
Instead, the SQL query that ended up being generated was this. Notice where the brackets are. It made our code send an email to people with messages on this Thankbox OR *everyone who has an account and has ever left a message anywhere*
But hey, our tests were passing. 5/7pic.twitter.com/k4qnrx9Wf3
1 odpowiedź 0 podanych dalej 1 polubionyPokaż ten wątek -
Luckily,
@JPritchard1987 caught this as soon as it happened and fixed it immediately. It was very simple - we just had to nest the queries. This put the brackets of the SQL query exactly where they should be. 6/7pic.twitter.com/0S0aDnEI0r
2 odpowiedzi 1 podany dalej 3 polubionePokaż ten wątek -
What's the lesson here? Eloquent is great but don't just assume what it's going to do. When in doubt, run toSql() on your query to make sure it's outputting what you expect it to. Also - write the test you should've written in the first place
7/7pic.twitter.com/b2UCbAn8kV
1 odpowiedź 0 podanych dalej 3 polubionePokaż ten wątek -
Wydaje się, że ładowanie zajmuje dużo czasu.
Twitter jest przeciążony lub wystąpił chwilowy problem. Spróbuj ponownie lub sprawdź status Twittera, aby uzyskać więcej informacji.