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

Installation: The default area for the default author shouldn't be "null" #2529

Closed
twiro opened this issue Dec 12, 2015 · 9 comments
Closed

Comments

@twiro
Copy link
Contributor

twiro commented Dec 12, 2015

While doing a fresh install of Symphony the first (default) author is created without the option to choose a "default area" that he's forwarded to after login.

So in "sym_authors" the row "default_area" simply says "null" after installation.

In my opinion this should be replaced with "/blueprints/sections/" because apart from the installation process the option to not have a default area isn't considered in the UI.

If the default author opens his author details the field Default Area in the fieldset Login Details will tell him that "Sections Index" is set as default area - which is not true until he actually resaves his author settings.

If he doesn't resave his author settings after installation his default area - as soon as at least one section exists - will become the alphabetically first section, while the selectbox in his author settings will still keep showing "Sections Index" because it's not prepared for not having a value and therefore simply shows the first available value.

This behavior is pretty irritating and has bugged me quite a few times in recent projects.

By the way: this problem only exists for the fist/default author because in the symphony backend you can't create other authors without setting a default area.

@brendo
Copy link
Member

brendo commented Dec 12, 2015

This change potentially means code can be removed from Administration::__buildPage which contains the logic for resolving the default section.

It was a best effort and wasn't meant to be annoying :)

@twiro
Copy link
Contributor Author

twiro commented Dec 12, 2015

_This change potentially means code can be removed from Administration::_buildPage which contains the logic for resolving the default section.

You're speaking about these lines, right?

Should I update my PR by also fixing this part or would you recommend to not change the behavior at all, but rather it's misleading representation in the UI? Which would mean allowing the Default Area-Selectbox in the Login Details of the Author Section to be empty and not display Sections Index as selected when it's actually not.

Though that would be the more complicated solution - and I don't see much speaking against the simpler one. What do you think?

It was a best effort and wasn't meant to be annoying :)

I bet it was ;) And "annoying" sounds way too harsh - I was just a little bit irritated... multiple times ;)

@nitriques
Copy link
Member

Should I update my PR by also fixing this part

Hum... I think we should still retain the default behaviour since ensembles may rely on it.

But yeah, redirecting to the section index would be ok too I guess.

Which would mean allowing the Default Area-Selectbox in the Login Details of the Author Section to be empty and not display Sections Index as selected when it's actually not.

Yes why not.

@twiro
Copy link
Contributor Author

twiro commented Dec 15, 2015

Hum... I think we should still retain the default behaviour since ensembles may rely on it.

How would Ensembles be affected?

Even if we'd remove the abovementioned lines the only thing that might happen - if an older symphony installation gets updated and the default author never actively chose a default area - is that after login he no more is forwarded to the alphabetical first section (which anyway never happened by intention), but to the section index instead (which then would be the new fallback for not having a default area).

If this possible change should be avoided, then the lines better stay in the code as a fallback - if not, they could be deleted.

Which would mean allowing the Default Area-Selectbox in the Login Details of the Author Section to be empty and not display Sections Index as selected when it's actually not.

Yes why not.

This would just be necessary if we could not agree upon directly setting "Section Index" as the "default area" at the installation process. It's the only possible moment where the default area can be set to "null" and I'd rather like to see this exception fixed, than updating the interface to explicitly allow for this edge case.

@nitriques
Copy link
Member

How would Ensembles be affected?

Nevermind, authors are not in ensembles.

It's the only possible moment where the default area can be set to "null"

We should update the database in that case. I do not think we need to do it for upgrades, but new install should not allow for null in that column.

Do you want to add new commits to #2530 in order to remove the lines ? This must wait for 2.7.x tho.

@nitriques nitriques self-assigned this Dec 15, 2015
@nitriques nitriques added this to the 2.7.0 milestone Dec 15, 2015
@nitriques
Copy link
Member

@twiro ping! :)

@twiro
Copy link
Contributor Author

twiro commented Dec 21, 2015

It's the only possible moment where the default area can be set to "null"

We should update the database in that case. I do not think we need to do it for upgrades, but new install should not allow for null in that column.

I was wrong about that - It might not be the common case, but for sure a developer can create authors before creating the first section. In that case the "default area"-selectbox gets deactivated and the value in the database will be "null".

If after that a section is created and the author revisits his author settings it will tell him that the first section is his "default area" - though in fact he still doesn't have a default area. So we do have the same irritating behavior here as with the default author... and we shouldn't change the database!

But as creating additional authors before creating the first section might be a very rare edge case I'd be fine with simply solving this issue for the default/first author for now.

Do you want to add new commits to #2530 in order to remove the lines ? This must wait for 2.7.x tho.

Yes, I'm gonna update my PR! Should I wait for a 2.7.X-Branch and submit a new PR against that Branch? Or simply update my current PR?

@nitriques
Copy link
Member

Please submit when ready ! 2.7.x will continue where 2.6.x left off, so either way the merge can happen. Thanks!

@twiro
Copy link
Contributor Author

twiro commented Jan 8, 2016

Updated my PR!

nitriques pushed a commit that referenced this issue Apr 12, 2016
* Set "Sections Index" as default area of default (first) author

Because not having a default area isn’t considered in the UI.

* Set "/blueprints/sections/" as default area fallback for developers

#2529

* Fix Whitespace
nitriques pushed a commit to DeuxHuitHuit/symphonycms that referenced this issue Apr 12, 2016
…onycms#2530)

* Set "Sections Index" as default area of default (first) author

Because not having a default area isn’t considered in the UI.

* Set "/blueprints/sections/" as default area fallback for developers

symphonycms#2529

* Fix Whitespace
jensscherbl pushed a commit that referenced this issue May 28, 2017
* Set "Sections Index" as default area of default (first) author

Because not having a default area isn’t considered in the UI.

* Set "/blueprints/sections/" as default area fallback for developers

#2529

* Fix Whitespace
nitriques pushed a commit that referenced this issue Jun 16, 2017
* Set "Sections Index" as default area of default (first) author

Because not having a default area isn’t considered in the UI.

* Set "/blueprints/sections/" as default area fallback for developers

#2529

* Fix Whitespace

Picked from 17d87a5
nitriques pushed a commit that referenced this issue Jun 16, 2017
* Set "Sections Index" as default area of default (first) author

Because not having a default area isn’t considered in the UI.

* Set "/blueprints/sections/" as default area fallback for developers

#2529

* Fix Whitespace

Picked from 17d87a5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants