You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Will take the first existing row. In most of DBMS (at least all relational one), without explicit clause the order is unpredictable. So if many rows match the underlying find_by* query, we can't predict which one will be chosen.
Expected behaviour
That's a situation devise_invitable can't handle. The current implementation may hide bugs on devise_invitable user's code base.
if many existing invitable row match, the invitation process must stop, through an exception (I believe).
I will submit a PR to change that behaviour soon 😃
I don't understand this issue. Why invite_key is not configured to be unique? Invite_key can be set to more than one column, in cases when one column, such as email, is not unique.
I don't understand this issue. Why invite_key is not configured to be unique? Invite_key can be set to more than one column, in cases when one column, such as email, is not unique.
Yes it should be configured to a unique column, but it is technically possible to do otherwise
And if we do so, devise_invitable proceed without any message / error even when multiple elements are returned, and this may go unnoticed and result in unexpected behavior
Summary
If
config.invite_key
is not configured to ensure a strict row unicity,invite!
may send invitation to a random user / account.Details
In models.rb L311, the instruction :
Will take the first existing row. In most of DBMS (at least all relational one), without explicit clause the order is unpredictable. So if many rows match the underlying
find_by*
query, we can't predict which one will be chosen.Expected behaviour
That's a situation
devise_invitable
can't handle. The current implementation may hide bugs ondevise_invitable
user's code base.if many existing invitable row match, the invitation process must stop, through an exception (I believe).
I will submit a PR to change that behaviour soon 😃
FYI @TristanBelin @bakster-jv
The text was updated successfully, but these errors were encountered: