Skip to content

Refs #3807: provide special admin login form if not logged in in admin context#514

Closed
jdalsem wants to merge 2 commits into
Elgg:masterfrom
jdalsem:#3807-special-admin-login
Closed

Refs #3807: provide special admin login form if not logged in in admin context#514
jdalsem wants to merge 2 commits into
Elgg:masterfrom
jdalsem:#3807-special-admin-login

Conversation

@jdalsem

@jdalsem jdalsem commented Feb 5, 2013

Copy link
Copy Markdown
Member

walled garden admin login form if access admin pages if not logged in

@jdalsem

jdalsem commented Feb 6, 2013

Copy link
Copy Markdown
Member Author

@cash this should make more sense and is more readable

@brettp

brettp commented Feb 21, 2013

Copy link
Copy Markdown
Member

I'm not sold on this use of the walled garden login index. Other thoughts?

@mrclay

mrclay commented Feb 25, 2013

Copy link
Copy Markdown
Member

@brettp is the the concern that using that function basically lies to the system and could introduce side effects?

@jdalsem

jdalsem commented Feb 26, 2013

Copy link
Copy Markdown
Member Author

@brettp the only other, more decent, option is to rewrite the admin layout, because that layout is not meant to be used for logged out users. It probably takes a few elgg_is_logged_in statements to fix those, and a bit of css/admin to make the login form look nice.

@mrclay

mrclay commented Feb 26, 2013

Copy link
Copy Markdown
Member

@jdalsem Agreed. 3rd party plugins may also extend the admin UI via hooks/extensions adding code that expects a logged in user, though I'm not sure how much we need to worry about that.

@cash

cash commented Feb 26, 2013

Copy link
Copy Markdown
Contributor

The better solution may be to move the admin views to their own viewtype. Then we can define a single column and a two column layout in the admin viewtype. The login page would use the single column layout.

We discussed this during 1.8 development. I don't remember the reason for not doing this.

@brettp

brettp commented Feb 26, 2013

Copy link
Copy Markdown
Member

@brettp is the the concern that using that function basically lies to the system and could introduce side effects?

Yes. It's using a very specific piece of code for something it wasn't meant for. Feels hacky.

We discussed this during 1.8 development. I don't remember the reason for not doing this.

See #3893. tl;dr: more work to keep views in sync.

@jdalsem

jdalsem commented Feb 28, 2013

Copy link
Copy Markdown
Member Author

Then first #3893 needs to be fixed before we can do this.

@cash

cash commented Jun 17, 2013

Copy link
Copy Markdown
Contributor

I'm closing this pull request. We can revisit in the future.

@cash cash closed this Jun 17, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants