Skip to content
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

Added host level setting for browser and upload folders of html editor #5409

Merged

Conversation

skamphuis
Copy link
Contributor

Closes #5408

Summary

Backgroup of the changes can be found in #5408
In summary I added 4 settings for host level, and implmented their usage on opening the file browser, and on uploading files.

@bdukes bdukes added this to the 9.11.1 milestone Nov 29, 2022
Copy link
Contributor

@david-poindexter david-poindexter left a comment

Choose a reason for hiding this comment

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

This looks good to me - thanks @skamphuis 🎉

@valadas
Copy link
Contributor

valadas commented Dec 6, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@valadas valadas changed the title Feature/5408 cke provider improvements Added host level setting for browser and upload folders of html editor Dec 6, 2022
Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

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

The code looks ok to me.

Just a couple of questions.
I understand that the host level settings here win, so say I have 20 portals with mixed folder settings, if I change the host level setting, then that changes it for all portals. Now let's suppose that some admins of these portals don't want that host-level folder or the host was accidentally playing with the feature...

  1. Is it right for the host setting to win over the portal level setting ? Or should it be a fallback instead of an override ?

  2. Assuming this is the desired behaviour, can it be reversed easily, like unsettling the host level folders ?

@valadas
Copy link
Contributor

valadas commented Dec 18, 2022

@skamphuis did you see my comments above ?

@skamphuis
Copy link
Contributor Author

@skamphuis did you see my comments above ?

Did now, thanks.

@skamphuis
Copy link
Contributor Author

The code looks ok to me.

Just a couple of questions. I understand that the host level settings here win, so say I have 20 portals with mixed folder settings, if I change the host level setting, then that changes it for all portals. Now let's suppose that some admins of these portals don't want that host-level folder or the host was accidentally playing with the feature...

The use case actually is a host that needs/wants to enforce these settings onto all portals, which I can imagine from a support point of view where the host is supporting many portals being administered by less-techy admins.

1. Is it right for the host setting to win over the portal level setting ? Or should it be a fallback instead of an override ?

It was intended for the host setting to win.

2. Assuming this is the desired behaviour, can it be reversed easily, like unsettling the host level folders ?

The host can obviously empty the settings again to reverse the behaviour. An admin has no power over that. We could add an extra setting, available for the host, to allow an admin to override. Is that what you meant?

@valadas
Copy link
Contributor

valadas commented Dec 19, 2022

We could add an extra setting, available for the host, to allow an admin to override. Is that what you meant?

Not necessarily, just wanted to know the intention here to make sure everyone is on the same page. This is fine for my use cases, I'll just hold on merging it to make sure the other reviewers understand this and are fine with it. Thanks for the update.

@dnnsoftware/approvers we may want to discuss this one on the next meeting just to make sure everyone is fine with this.

@donker donker merged commit 4be2431 into dnnsoftware:develop Dec 20, 2022
zyhfish pushed a commit to zyhfish/Dnn.Platform that referenced this pull request Dec 29, 2022
dnnsoftware#5409)

* added host level folder settings

* implemented host-level browser and upload folders
@bdukes bdukes mentioned this pull request Feb 7, 2023
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CKEditor host level setting for browser and upload folders
6 participants