Skip to content

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Sep 4, 2020

Currently the Additional settings for personal settings only shows when there are legacy panel registered via \OCP\App::registerPersonal('ransomware_protection', 'personal');
When this is transformed into a "normal" setting, it works and shows up in the additional section, as long as there is one "old" setting registered, otherwise the menu entry will not show up (but the page will work and show the setting).

So we now check if there is a legacy setting or any other setting registered for additional to determine whether the panel should display, or was this a conscious decision to get rid of the additional section for personal settings but keep it in administration settings?

@dassio
Copy link

dassio commented Sep 7, 2020

When this is transformed into a "normal" setting, it works and shows up in the additional section, as long as there is one "old" setting registered, otherwise the menu entry will not show up (but the page will work and show the setting).

this is because there is no an personal addtional section in the settings app, shouldn't we add the section instead of hacking the system ? it will make later comer much eaiser to understand the structure.

please check my pull request #22144

@blizzz blizzz merged commit 3eb748f into master Sep 7, 2020
@blizzz blizzz deleted the bugfix/noid/allow-additional-personal-settings-via-normal-registration branch September 7, 2020 09:34
@nickvergessen
Copy link
Member Author

this is because there is no an personal addtional section in the settings app, shouldn't we add the section instead of hacking the system ? it will make later comer much eaiser to understand the structure.

While I agree in general, the one liner is much easier to be backported. So let's make it properly (like in the admin section?) with your PR, and only do the minimal impact PR for the backporting

@nickvergessen
Copy link
Member Author

/backport to stable19

@nickvergessen
Copy link
Member Author

/backport to stable18

@dassio
Copy link

dassio commented Sep 9, 2020

this is because there is no an personal addtional section in the settings app, shouldn't we add the section instead of hacking the system ? it will make later comer much eaiser to understand the structure.

While I agree in general, the one liner is much easier to be backported. So let's make it properly (like in the admin section?) with your PR, and only do the minimal impact PR for the backporting

already closed my PR, this much simpler

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.

5 participants