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

Administration #725

Merged
merged 1 commit into from Mar 30, 2022
Merged

Administration #725

merged 1 commit into from Mar 30, 2022

Conversation

Drumor
Copy link
Collaborator

@Drumor Drumor commented Oct 8, 2021

This is a first implem of administration part. This lets admins create/delete/activate users. It's also possible to remove bindings for specific users.

Copy link
Member

@anthonygego anthonygego left a comment

Choose a reason for hiding this comment

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

I think this implementation can be lighter by removing some useless pieces of code.

There is an issue with displaying the bindings: the first one is displayed each time.

The performances in case of +2k users are terrible. Please add some random users in your local DB to test, and potentially do some pagination.

inginious/frontend/templates/admin/admin_users.html Outdated Show resolved Hide resolved
inginious/frontend/templates/admin/admin_users.html Outdated Show resolved Hide resolved
inginious/frontend/templates/layout.html Outdated Show resolved Hide resolved
inginious/frontend/pages/admin/admin.py Outdated Show resolved Hide resolved
inginious/frontend/pages/admin/admin.py Show resolved Hide resolved
inginious/frontend/pages/admin/admin.py Outdated Show resolved Hide resolved
inginious/frontend/pages/admin/admin.py Outdated Show resolved Hide resolved
inginious/frontend/pages/admin/admin.py Outdated Show resolved Hide resolved
@Drumor Drumor force-pushed the administration branch 2 times, most recently from 68dda6a to 632f046 Compare October 21, 2021 09:40
@anthonygego
Copy link
Member

Performance for +2k users are still terrible. A pagination system should be implemented. Maybe some parts of the submissions pagination mechanism can be refactored for other pages.

inginious/frontend/pages/admin/admin.py Show resolved Hide resolved
inginious/frontend/pages/admin/admin.py Outdated Show resolved Hide resolved
inginious/common/base.py Outdated Show resolved Hide resolved
inginious/frontend/pages/admin/admin.py Outdated Show resolved Hide resolved
inginious/frontend/user_manager.py Outdated Show resolved Hide resolved
inginious/frontend/pages/utils.py Outdated Show resolved Hide resolved
inginious/frontend/templates/admin/admin_users.html Outdated Show resolved Hide resolved
inginious/frontend/user_manager.py Show resolved Hide resolved
inginious/frontend/user_manager.py Outdated Show resolved Hide resolved
inginious/frontend/user_manager.py Outdated Show resolved Hide resolved
inginious/frontend/pages/course_admin/danger_zone.py Outdated Show resolved Hide resolved
inginious/frontend/pages/utils.py Outdated Show resolved Hide resolved
inginious/frontend/templates/admin/admin_users.html Outdated Show resolved Hide resolved
Copy link
Member

@anthonygego anthonygego left a comment

Choose a reason for hiding this comment

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

Stay strong, this is almost done.

inginious/frontend/user_manager.py Outdated Show resolved Hide resolved
inginious/frontend/pages/admin/admin.py Outdated Show resolved Hide resolved
inginious/frontend/pages/admin/admin.py Show resolved Hide resolved
inginious/frontend/pages/admin/admin.py Outdated Show resolved Hide resolved
inginious/frontend/user_manager.py Outdated Show resolved Hide resolved
inginious/frontend/user_manager.py Outdated Show resolved Hide resolved
inginious/frontend/templates/admin/admin_users.html Outdated Show resolved Hide resolved
inginious/frontend/templates/admin/admin_users.html Outdated Show resolved Hide resolved
inginious/frontend/templates/admin/admin_users.html Outdated Show resolved Hide resolved
inginious/frontend/static/js/admin.js Show resolved Hide resolved
Copy link
Member

@anthonygego anthonygego left a comment

Choose a reason for hiding this comment

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

When you revoke a binding and get an error message for whatever reason, this error message is not cleaned when the modal box is closed.

Printing it in a red alert box instead of just displaying the message black on white would also be nice to better catch the user attention.

Copy link
Member

@anthonygego anthonygego left a comment

Choose a reason for hiding this comment

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

This should be good after this last fix. Please also clean your commits. This is a bit too big to be squashed in a unique commit.

inginious/frontend/static/js/admin.js Outdated Show resolved Hide resolved
Copy link
Member

@anthonygego anthonygego left a comment

Choose a reason for hiding this comment

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

Sorry but I can't merge this with such a security issue remaining.

inginious/frontend/user_manager.py Outdated Show resolved Hide resolved
Copy link
Member

@anthonygego anthonygego left a comment

Choose a reason for hiding this comment

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

LGTM. Can you squash the current changes and update the minified js file ?

Administration module - first features : Activate and delete a user

Administration module - create user feature preparation

Administration module - create user feature

Administration module - base of bindings

Administration module - base of bindings

Administration module - base of bindings

Admin - End revoke binding feature

Admin - Use get_home_path for url call and clean html building

Admin - makes template fit with already used system.

Admin - rework INGIniousAdministratorPage

Admin - fix ugly JS issue

Admin - Some rework + INGIniousAdministratorPage review

fix binding loop

simplify workflow

simplify workflow

Avoid superadmin bypass

remove add specific page - replace with modal

make request of bindings more secure

adding user paging based on submissions paging system

move all hash generation to a method

move all hash generation to a method

solve superadmin check bypass

fix double hash of password input

make codacy happier

optimize imports + give feedback

rename and move hash password function

move hash password function

polish activate_user method

Simplify feedback

Update feedback

improve users fetch

move Marketplace link

fix pagination center

check activate status for several users

Move JS into a dedicated file.

Simplify ajax requests

Move JS logic into dedicated file.

improve feedback

optimize imports

review feedback

use hash_password as class method

remove useless verification in template

display feedback when binding is not deletable

Fixing remarks

Handling all actions + gettext

Adding "activated" field to UserInfo + Clean duplicate code +

use dict comprehension

Update docstring for get_users_info

Set a behaviour for unknown action.

Alert feedback more readable.
fix bug with Keyerror
Clean feedback on closing modal

Set feedback display in a else statement

Review user deletion and improve security.

minimize list of selected courses to improve request and avoid bad logging.

Minify Js
@anthonygego anthonygego merged commit fd9e5b0 into master Mar 30, 2022
@anthonygego anthonygego deleted the administration branch March 30, 2022 08:42
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.

None yet

2 participants