Bug 11534 - impossible to remove instructors on course reserve
Summary: impossible to remove instructors on course reserve
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Course reserves (show other bugs)
Version: Main
Hardware: All All
: P3 major (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-13 19:31 UTC by Marjorie Barry-Vila
Modified: 2014-12-07 20:05 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
link remove instructor course reserve (124.76 KB, image/png)
2014-01-29 17:08 UTC, Marjorie Barry-Vila
Details
Bug 11534 - impossible to remove instructors on course reserve (4.98 KB, patch)
2014-01-29 19:41 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 11534 - impossible to remove instructors on course reserve (4.99 KB, patch)
2014-01-30 05:26 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 11534 - impossible to remove instructors on course reserve (5.04 KB, patch)
2014-01-30 19:52 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marjorie Barry-Vila 2014-01-13 19:31:28 UTC
Hi,

On editing a course reserve, i can not remove an instructor. When i click on remove, nothing happens.


Someone has already tested it?

Marjorie
Comment 1 Liz Rea 2014-01-14 06:53:45 UTC
Hi Marjorie,

I was able to do this in Master and the instructor was removed correctly. What browser and OS are you using?

Cheers,
Liz
Comment 2 Marjorie Barry-Vila 2014-01-14 13:21:15 UTC
Hi Liz,

I'm using firefox 26.0 and windows 7.

I also tested on google chrome 31 et IE11 but it does not work.

I tested also on http://intranet.bywatersolutions.com but it does the same, i can not remove instructors. 

An idea?

Thanks,

Marjorie
Comment 3 Owen Leonard 2014-01-14 14:45:08 UTC
I can reproduce this bug in FF26 on Win7, also in FF26 and Chromium on Ubuntu. Nothing happens when I click the "remove" link but no JavaScript errors are reported.
Comment 4 Owen Leonard 2014-01-14 14:53:18 UTC
A card number is passed to RemoveInstructor() in order to remove a the <div> containing the relevant field:

$( '#borrower_' + cardnumber ).remove();

...but the <div> looks like this:

<div id="borrower_0011960">

...and the number passed to the function is this:

11960

Could the JavaScript engine in Liz's browser have different type handling?
Comment 5 Liz Rea 2014-01-17 10:08:09 UTC
Hmm, mine was Chromium and Firefox (latest on both) in Ubuntu, and it still worked in both. I tried with long and short cardnumbers, I'm just not seeing it. But since you guys did, I'll believe you that it exists. :)

Liz
Comment 6 Owen Leonard 2014-01-29 16:27:49 UTC
Marjorie do your patron card numbers have leading zeroes? I suspect that's why Liz can't reproduce the error but I can.
Comment 7 Marjorie Barry-Vila 2014-01-29 17:08:49 UTC
Created attachment 24869 [details]
link remove instructor course reserve
Comment 8 Marjorie Barry-Vila 2014-01-29 17:10:18 UTC
Hi Owen,

No there is no zeroes in patron card number.

See the capture for the link when i do a hover on "remove".
Liz, do you see the same?

Thanks,

Marjorie
Comment 9 Owen Leonard 2014-01-29 19:41:41 UTC Comment hidden (obsolete)
Comment 10 Marjorie Barry-Vila 2014-01-29 19:55:05 UTC
Owen,

I understood why i did not be able to remove my instructors.

There is no number in the cardnumber I used, there is only letters...
I tested it with a cardnumber with numbers (without leading zero) and it works!

So, a cardnumber can contain letters? 

Marjorie
Comment 11 Owen Leonard 2014-01-29 20:11:30 UTC
(In reply to Marjorie Barry-Vila from comment #10)
> There is no number in the cardnumber I used, there is only letters...

That's good to know. My patch should fix the problem for both of us.
Comment 12 Chris Cormack 2014-01-30 05:26:18 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2014-01-30 19:52:35 UTC
Created attachment 24904 [details] [review]
Bug 11534 - impossible to remove instructors on course reserve

In some cases clicking the "remove" link to remove a course instructor
when editing a course reserves course doesn't work. I suspect this is
caused by leading zeroes in the card number. This patch refactors the
relevant function to fix the problem and to remove inline JavaScript
from generated markup.

Other changes:

- Make the "Remove" text translatable
- Re-order the first name and last name when added by JavaScript to
match the template's "surname, firstname" order.

To test:

1. If necessary, create or modify a patron to have a card number
   with leading zeroes.
2. Go to Course reserves and add or edit a course.
3. If you are editing a course and there are existing instructors, click
   the "remove" link. Nothing will happen.
4. Perform a patron search and choose one of the results. The name
   should be added to the list of instructors in the format "surname,
   firstname."
5. Click the "remove" link next to the patron name which was just added.
   Nothing will happen.
6. Apply the patch and repeat step 4. The "remove" link should work.
7. Add an instructor and save the course reserve. Repeat steps 2 and 3.
   The "remove" link should now work.

To test the translation fix, after applying the patch run "translate
update [language code]" and confirm that the "Remove" string is listed
in the updated po file for course_reserves/course.tt.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 14 Galen Charlton 2014-01-31 15:46:59 UTC
Pushed to master.  Thanks, Owen!
Comment 15 Fridolin Somers 2014-04-23 16:29:16 UTC
Patch pushed to 3.14.x, will be in 3.14.06