search

chamilo / chamilo-lms

Chamilo is a learning management system focused on ease of use and accessibility
https://chamilo.org
GNU General Public License v3.0
798 stars 480 forks source link

Tickets: select course or session by default #2607

Closed ywarnier closed 6 years ago

ywarnier commented 6 years ago

Current behavior

When the ticket floating tab is enabled, the "Report issue" link is filled with the course code and the session ID, if we are in a course or a session context. The idea of that info is to be able to pre-fill the ticket reporting form so that we don't have to indicate in which course or in which session we were when reporting the ticket. We can still change those values, but by default they are pre-selected.

In the ticket system, there are two issues right now. One is that the default course selection mechanism doesn't work. It seems like, because of the context of the ticket code, the $_SESSION['_cid'] is emptied before getting to the right line in new_ticket.php. So a call to api_get_course_info() just returns an empty array, even though the URL shows cidReq=XYZ.

https://github.com/chamilo/chamilo-lms/blob/696c9c007cbc5afd262e4aba7ac97368af870c40/main/ticket/new_ticket.php#L372-L388

Instead, calling api_get_course_info($_GET['cidReq']) after checking if $_GET['cidReq'] is not empty fixes the issue of knowing in which course we are.

Then the second issue is that, even though the right $param['course_id''] default is set in the form, this doesn't show the pre-selected course. There seems to be an issue with the type of drop-down select box that we use, that prevents the selection by default.

I believe I had previously reported that the link to report a ticket (and the one under the icon to create one) was not including the course code, and that has been fixed, but I'm pretty sure at this point this also enabled the possibility to pre-select the course (otherwise I wouldn't have asked for it).

Could you please fix the default selection? There seems to be something of a mixed process to fill the select box when you search or when you have a session, and to pre-set a default course.

Also (this is a third problem and could be resolved in a separate issue if too long to fix here), if there are no session on the system or no session in which the user is registered (which is already checked by a call to SessionManager::get_sessions_by_user()), then there's no need to show the session drop-down box, I think (unless the user is admin, OK, that might be an exception).

Steps to reproduce

Chamilo Version

1.11.x 20180724

NicoDucou commented 6 years ago

There is an other problem, if in app/config/configuration.php we do not define the variable ticket_project_user_roles then only the admin have access to the ticket system.

// Allow ticket projects to be access by specific chamilo roles /$_configuration['ticket_project_user_roles'] = [ 'permissions' => [ 1 => [17] // project_id = 1, STUDENT_BOSS = 17 ] ];/ //

I'm not sure if it's the right way to do. I think if the variable is not defined then every user role should have access to the ticket system, and if a restriction is defined then only the user role defined should have access.

For the moment on 11.chamilo.org only the admin has access and I can not change the configuration in app/config/configuration.php

I will test on my local plateform.

NicoDucou commented 6 years ago

I just tested and the 3 issues are solved :

jmontoyaa commented 6 years ago

Yes, that's how it works, the ticket system was created only for admins at first.

Then that "role" option (ticket_project_user_roles) was added to allow specific roles.

NicoDucou commented 6 years ago

Ok, so I just leave this issue opened for @ywarnier to see if he is ok with this way with only admin or if he wants to change it as I proposed 3 comments above

ywarnier commented 6 years ago

To my knowledge no, it is not how it works (not sure where the opposite concept came from but its origins are old already so... maybe). The ticketing system, when enabled, should be accessible to everybody except if roles are defined, then it should be restricted to those roles. Thanks for pointing it out. I can take that part in charge, I think.

ywarnier commented 6 years ago

As far as I can see the current behaviour works like this: if ticket_project_user_roles is not set, you have access to the tickets systems but only to the tickets you reported or that were assigned to you, so that looks fine to me...

https://github.com/chamilo/chamilo-lms/blob/beae46a41c6edbc2188488d50464c0744ae35617/main/inc/lib/TicketManager.php#L1032-L1041

It's diffrent with userIsAllowInProject() though: https://github.com/chamilo/chamilo-lms/blob/beae46a41c6edbc2188488d50464c0744ae35617/main/inc/lib/TicketManager.php#L2422-L2438

But that's to give access to a project. I'm not sure we need it if it's only to create a new ticket, though.

ywarnier commented 6 years ago

I see what you mean: I don't see the tab if I'm a normal user. This is due to this check: https://github.com/chamilo/chamilo-lms/blob/beae46a41c6edbc2188488d50464c0744ae35617/main/inc/lib/template.lib.php#L1284-L1296

So yes, indeed, there is an issue here... and a complex one. I will change the behaviour of this then, to allow anyone in project 1 (the default initial project) unless there is a specific restriction of roles for it.

ywarnier commented 6 years ago

I still have an issue with the list of courses on a portal with 18K courses. The course does not appear pre-selected and the list of courses does not appear at all (even when seaching). Trying to understand what the issue is...

ywarnier commented 6 years ago

In the latter case, I get a gateway timeout on the call to session.ajax.php?a=get_courses_inside_session&session_id=0

ywarnier commented 6 years ago

I'm closing this task. The issue is due to the long time it takes to generate the list of courses.

ywarnier commented 6 years ago

OK so I understood the issue a bit better and sent another commit (85ef745825150efca772b22ad0340bf9e53afc14). The filter on the project permissions is meant to prevent users to see tickets from other users. If you are "allowed" in a project, then you see all the tickets of that project, even if they're not yours. The issue with that is that the check was also used to decide whether we show the floating ticket tab or not (the one on the right side), and that means that, if you don't have access to project 1, then you don't see the floating tag, which makes no sense to me.

So the new solution is: keep tickets filtered unless you have an explicit permission on the whole project (from configuration.php) BUT always show the issue reporting (floating) tab if that's been enabled in settings_current. Now the whole behavior makes sense.