New feature #19201: Add column "active" to users, to be able to activate or deactivate users #3550
Conversation
|
My opinion : seems to be create event for only one plugin. Can be great if we can use for other purpose |
|
OK, i think it's a PR to add a core plugin for Cloud version. But it's updating CE version for cloud version … Seems strange for me. |
Yea, please wait, we'll clean this up. ^^ |
olleharstedt
changed the title
|
Modified the title of the PR. |
|
Next step would be to remove everything LS Cloud related from this PR and only keep functionality related to the new feature activate/deactivate user. :) |
application/views/userManagement/partial/confirmuseractivation.php
Outdated
Show resolved
Hide resolved
|
Merged develop into this branch again and updated dbversion to 619. |
| App()->getPluginManager()->dispatchEvent($oEvent); | ||
| $data = $oEvent->get('data'); | ||
|
|
||
| foreach ($data as $key => $item) { |
There was a problem hiding this comment.
Usage of $event->append( for plugins
See https://manual.limesurvey.org/BeforeToolsMenuRender
Warning : broke if data is null with php8.1. (remind to activate debug for fix and new feature).
| @@ -498,6 +498,14 @@ public function getAdminFooter($url, $explanation, $return = false) | |||
| $aData['versiontitle'] = gT('Version'); | |||
| } | |||
|
|
|||
| $oEvent = new PluginEvent('beforeFooterRender'); | |||
There was a problem hiding this comment.
This event is perhaps not needed if you use asset manager to register a script with current_plan instead?
https://github.com/LimeSurvey/cloud_client_limesurvey/pull/24/files#r1415473939
There was a problem hiding this comment.
@mohabmes Can you comment on why you marked this as resolved?
There was a problem hiding this comment.
@olleharstedt Registering a script with the current_plan at this point serves no purpose; we only wanted that event to transmit the current plan for the partial rendering of the appropriate plan upgrade modal, displaying the accurate next bigger plan. (no JS involved).
There was a problem hiding this comment.
Ah ok. In that case, it's probably better if the beforeFooterRender spits out HTML instead of a data array, so that other plugins can insert whatever HTML they want without changing any LS core view files.
That also means that the view file planupgrade.php can be moved to the LS Pro plugin folder instead of the core view folder.
There was a problem hiding this comment.
Compare with this event: https://manual.limesurvey.org/BeforeCloseHtml (only available in survey taking view, afaik; @Shnoulle you can confirm?).
So you can add a similar event to admin view called beforeCloseAdminHtml.
There was a problem hiding this comment.
beforeCloseHtml never work good … you can just use it for registering script.
Have a real way to add or update element is better.
There was a problem hiding this comment.
Have a real way to add or update element is better.
Like menu classes or? Hard to see how that would work with something general.
There was a problem hiding this comment.
Menu class : yes, it's interesting.
With public : we can replace or add .twig view, but need better too … with a way to really use twig block and extend
There was a problem hiding this comment.
Seems overkill for just adding some footer HTML, or?
There was a problem hiding this comment.
Seems overkill for just adding some footer HTML, or?
I speak in general here :)
…hub.com/LimeSurvey/LimeSurvey into ls6/cr-1343/limit-creation-of-new-users
| @@ -171,7 +178,7 @@ public function newUserSession() | |||
| $this->setAuthFailure(self::ERROR_USERNAME_INVALID); | |||
| return; | |||
| } | |||
| if ($user->isExpired()) { | |||
| if ($user->isExpired() || (isset($user->user_status) && !$user->user_status)) { | |||
There was a problem hiding this comment.
I think this check has to be moved to
application/core/LSUserIdentity.php
71 $authEvent = new PluginEvent('newUserSession', $this); // TODO: rename the plugin function authenticate()
Basically, never fire up the event "newUserSession" if username belongs to a non-active user.
Remember that the non-active check has to work for authentication plugins that are not in the git repository.
There was a problem hiding this comment.
Must be moved to
Else you have same issue than : https://bugs.limesurvey.org/view.php?id=19117
PS : @olleharstedt #3607 still not reviewed.
There was a problem hiding this comment.
Ah yeah, I did think about how to achieve that.
There was a problem hiding this comment.
But then we get the same issue with updates, or?
There was a problem hiding this comment.
But then we get the same issue with updates, or?
Yes, but need to be fixed on the same way :)
There was a problem hiding this comment.
https://bugs.limesurvey.org/view.php?id=19304 wrote down your issue here
|
Someone need to report the issue with user keeping session when set as inactive ! |
No description provided.