Skip to content

Fixed: disabled Category EULA box when no Default EULA created #16913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

akemidx
Copy link
Member

@akemidx akemidx commented May 12, 2025

fixes: SC-29119

EULA on the Category settings page for each category was disabled while there was no default EULA made, even when there was a EULA specifically set on the Category itself.

This makes it so that EULA is always editable, and if there is no Default EULA, then it will uncheck and grey out the "use Default EULA" option

Screenshot 2025-05-12 at 8 18 02 AM Screenshot 2025-05-12 at 8 18 08 AM

@akemidx akemidx requested a review from snipe as a code owner May 12, 2025 12:19
@akemidx akemidx requested review from marcusmoore and removed request for snipe May 12, 2025 12:19
Copy link
Collaborator

@marcusmoore marcusmoore left a comment

Choose a reason for hiding this comment

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

Hm...this isn't exactly working right. It doesn't handle the case where a category is set to use the default eula but the default eula is not set.

For example, without a global EULA set while editing this category:
image

The "Use default eula" is correctly not checked but once I remove the category eula it becomes checked/disabled.

example

Also the notice continues to say " An email will be sent to the user because the global EULA is being used."

type="checkbox"
name="use_default_eula"
value="0"
wire:model.live="shouldUncheckDefaultEulaBox"
Copy link
Collaborator

Choose a reason for hiding this comment

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

wire:model should be used for properties and not event listeners/handlers.

I think ensuring $useDefaultEula is accurate is the correct fix?

Maybe we can make sure we're in a valid state from the start from within the mount method?

Copy link
Member Author

Choose a reason for hiding this comment

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

I see this. For some reason i probably didn't do that combo of actions, cause I don't remember that happening.

Additionally, the checkmark for "use default EULA" persists on the category index page, even when no default EULA is made. so that is another spot we'd have to fix as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

or do we want to leave it there, but make sure the checkbox is always uncheckable if its checked, but then we don't allow it to be rechecked?

Copy link
Member Author

Choose a reason for hiding this comment

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

so, yea, that checkmark WILL change upon another save of the edit page. so its not a huge issue, but could be confusing.

if there is no default eula set, then useDefaultEula should always be false. At no point it should ever turn true, I don't think it ever does.

So, it might just be a mess with reading the correct true/false and then presenting it on the page.

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.

2 participants