Project

General

Profile

Refactoring #3155

Admin User browser broken

Added by Nathalie Blocry about 10 years ago. Updated almost 9 years ago.

Status:
Bug resolved
Priority:
Normal
Assignee:
Hans De Bisschop
Start date:
28/03/2011
Due date:
% Done:

0%

Estimated time:

Description

the fix for issue #2462 creates another bug.
when the weblcms is not installed the userbrower is not displayed because user-data-manager.class.php calls weblcms-specific code on line 121

(in DEV and STABLE)

History

#1

Updated by Hans De Bisschop about 10 years ago

  • Project changed from Chamilo LCMS Connect to Admin
  • Category deleted (5)
#2

Updated by Hans De Bisschop about 10 years ago

  • Target version set to 2.1.0
#3

Updated by Anonymous about 10 years ago

  • Status changed from New to Assigned
  • Assignee set to Anonymous
#4

Updated by Anonymous about 10 years ago

  • Status changed from Assigned to Needs testing
  • % Done changed from 0 to 100

I can't seem to reproduce the problem, but remember having it myself... I've added some registration checks in the user_data_manager so the error should be avoided if it ever turned up again... the only thing that can happen is the checks won't work or the use statements fail if the files are actually missing because of the use of constans for application name etc.

#5

Updated by Anonymous about 10 years ago

checks added on stable

#6

Updated by Nathalie Blocry about 10 years ago

These checks will probably prevent the error from occurring again; but normally all references to optional applications should be removed from the core application imho.

The weblcms should be able to gracefully handle the cases where a course-admin is deleted.
deleting users can be/is problematic for all applications so we should avoid a future situation where we could have 300 "is_installed" checks like this in core packages.

#7

Updated by Stefaan Vanbillemont about 10 years ago

  • Status changed from Needs testing to Bug resolved
#8

Updated by Nathalie Blocry about 10 years ago

  • Tracker changed from Bug to Refactoring
  • Status changed from Bug resolved to Needs more info

I don't think this can be marked as resolved.
there is still code from an optional application being executed in the core.
although it does not throw an error since a check is added, this doesn't mean it is really resolved

/** * Checks whether the user is allowed to be deleted * Unfinished.
*/
static function user_deletion_allowed($user) {
//A check to not delete a user when he's an active teacher
// Weblcms is an optional application, so first check if the app is installed:
$weblcms_registration = AdminDataManager :: get_registration(WeblcmsManager :: APPLICATION_NAME);
if($weblcms_registration) {
// if active
if($weblcms_registration->get_status() == Registration :: STATUS_ACTIVE) {
$courses = WeblcmsDataManager :: get_instance()->count_courses(new EqualityCondition(Course :: PROPERTY_TITULAR, $user->get_id()));
if ($courses > 0) {
return false;
}
}
}
return true;
}

#9

Updated by Anonymous about 10 years ago

  • Status changed from Needs more info to New
  • Target version changed from 2.1.0 to 28

I don't see any way to do this check otherwise at the moment. You should not be allowed to delete the user if he is an active teacher, making the weblcms responsible for removing all linked data on removal is in my opinion unacceptable, because important data could be removed, and the other way around, preventing the user app from removing, is, well, just as it states now, responsibility of the core.

Maybe some generic system should be implemented that asks all installed apps if the operation should be performed, this way each application can do some checks and possibly prevent the deletion. But since we are trying to get something stable and this is a major refactoring it might not be a bad idea to leave this "resolved" for the time being, and change the tracker to refactoring (in a later release with a generic system for making those checks in all apps).

Any other remarks?

#10

Updated by Hans De Bisschop about 10 years ago

This is as a matter of fact not "resolved" at all.

I agree with Nathalie that the current solution is not a solution, it's not even one that will always work. (just try and delete the WebLCMS folder ... you might not be able to use the WeblcmsManager class then)

The WebLCMS (or any other optional app) should always be responsible for gracefully handling what should be deleted when a user gets deleted. On top of that it should even be able to veto such a delete if and when appropriate. This simply implies implementing a system similar to the deletion of content objects for users. (and per extension we also need that for groups or any other kind of global entity that could be used locally)

#11

Updated by Anonymous about 10 years ago

  • % Done changed from 100 to 0
#12

Updated by Anonymous about 10 years ago

  • Assignee deleted (Anonymous)
#13

Updated by Stefaan Vanbillemont about 10 years ago

  • Target version changed from 28 to Backlog (default)
#14

Updated by Stefaan Vanbillemont almost 9 years ago

  • Status changed from New to Bug resolved
  • Assignee set to Hans De Bisschop
  • Target version changed from Backlog (default) to LCMS 3

Resolved

Also available in: Atom PDF