[FD-50375] Category-edit screen shows "Send email to user on checkin/checkout" is checked but it's not

[uberbrady]

IF you look at the category listing, you'll see that there is no check on the "Send email to user on checkin/checkout" checkbox.

But when you click in to the Category, it looks like it's already checked (even though, in reality, it's not):

If you click 'Save' it will save the checkmark correctly however.

The way I'm thinking we should fix this is to make that field no longer 'disabled' when you type in a category-based EULA. I'm fine with default-checking it once a EULA has been input, but if it's not yet checked, we should show it that way. I think it should also be permissible to leave that box unchecked, which should make the EULA be sent on checkout, but not on checkin.

[snipe]

Is this possibly related to #16913?

[uberbrady]

Could be? Seems like a slightly different problem, though, and might be fixable from the same place.

[marcusmoore]

The reasoning for the current behavior is in #13407. Specifically Additional Notes section:

Previously, the checkin_email property in the database for categories was not always accurate since it was possible that an email would be sent due to the conditions mentioned above. Now, if the checkbox is programmatically selected the value is updated in the database as well.

Users were confused why emails were getting sent when that checkbox was unchecked. My intention was to accurately show that since there is a EULA set (1) we display a message (2) essentially saying "it doesn't matter if the checkbox above (3) is checked because an email is going to be sent either way so we'll just check and disable it for you now.

You mentioning clicking "Save" will save the property correctly and is what I meant by "if the checkbox is programmatically selected the value is updated in the database as well" from above but maybe I should have made a migration to update categories so checkin_email is accurate?

---

I think it should also be permissible to leave that box unchecked, which should make the EULA be sent on checkout, but not on checkin.

This is interesting because the field is actually called checkin_email and I think the original intention was for checkin emails and not checkout emails. We could add a new property, checkout_email to handle this and also include a migration that accurately updates both properties to true if the category has EULA text or uses the default EULA. That would maintain the current behavior for users and allow them to update their category settings after the setting was introduced.

I'll revisit before starting work but this is the implementation I think I'll start on.

[uberbrady]

My instinct here is to make the checkbox just a normal checkbox that reflects the state of the database, and treat it 'normally'. I think the messaging in (2) is enough to explain that, yes, if you're sending EULA's, checkout emails will happen, whether or not that box is checked. But the checkbox does (right now) seem to affect whether or not checkin emails will fire - regardless of a EULA being set or not. And I see no need to override that functionality; I could imagine it being useful in some cases.

(And, you are correct, if you had done a migration to flip the checkbox in the DB, that would've probably avoided the problem - but, again, I feel like that would nerf the feature that allows for email on checkout but not checkin).

Here's another option that might make sense - since we're already in Livewire here (if memory serves) - we could dynamically change the description of the checkbox to say 'Send email to user on checkin' when a EULA is already set? That, along with the messaging from (2) makes it pretty clear what's going on here. I don't know if that's worth creating another string, but I think it's at least a little clearer as to what's happening here.