This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs.
Created attachment 29105 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
Created attachment 30456 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
Created attachment 31567 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
Created attachment 35104 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
In my test, I applied the patch and ran updatedatabase.pl. However I received this error message: DBD::mysql::db do failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 13 at ./updatedatabase.pl line 9735. DBD::mysql::db do failed: Can't create table 'koha_kohadev.#sql-a48_3a' (errno: 150) at ./updatedatabase.pl line 9776. DBD::mysql::db do failed: Can't create table 'koha_kohadev.#sql-a48_3a' (errno: 150) at ./updatedatabase.pl line 9782. DBD::mysql::db do failed: Table 'koha_kohadev.club_templates' doesn't exist at ./updatedatabase.pl line 9801. DBD::mysql::db do failed: Can't create table 'koha_kohadev.#sql-a48_3a' (errno: 150) at ./updatedatabase.pl line 9806. DBD::mysql::db do failed: Can't create table 'koha_kohadev.#sql-a48_3a' (errno: 150) at ./updatedatabase.pl line 9811. Upgrade to 3.19.00.XXX done (Bug 12461 - Add patron clubs feature)
Created attachment 36098 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
Created attachment 36099 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
Created attachment 36192 [details] [review] Bug 12461 - Add patron clubs feature Easy conflict on updatedatabase.pl fixed.
We've tested it on sandbox. The patch apply correctly, but we don't see the "Patron clubs" link.
I do not see see the "Patron clubs" link either. It seems that changes to /prog/en/modules/tools/tools-home.tt are missing. They were e.g. in Attachment #30456 [details] (Going to the page directly with /cgi-bin/koha/clubs/clubs.pl works)
Created attachment 38748 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
Sorry, patch does not apply. Small conflicts, and DB update is not on atomicupdate, perhaps RM can make an exception
Created attachment 39181 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
Almost all tested, looks good. But reloading kohastructure with your patch applied, gives mysql kohadev < ~/kohaclone/installer/data/mysql/kohastructure.sql ERROR 1005 (HY000) at line 3656: Can't create table 'kohadev.#sql-42d1_401' (errno: 150) Can't understand or find the error
(In reply to Bernardo Gonzalez Kriegel from comment #14) > Almost all tested, looks good. > But reloading kohastructure with your patch applied, gives > > mysql kohadev < ~/kohaclone/installer/data/mysql/kohastructure.sql > ERROR 1005 (HY000) at line 3656: Can't create table 'kohadev.#sql-42d1_401' > (errno: 150) > > Can't understand or find the error Not to mention, why add the constraints separately?! If this was $sth = $dbh->prepare(), $sth->execute() method of doing this, then wouldn't likely get this issue either.
Additionally, this patch has a conflict with circ/circulation.pl now.
Created attachment 43428 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
Updated patch merges constraints into table creation, fixes merge conflicts. Tested with fresh install and upgrade!
Hi, I tried testing this in two different sandboxes and I get this error. Some problems occurred applying patches from bug 12461: <h1>Something went wrong !</h1>Applying: Bug 12461 - Add patron clubs feature Using index info to reconstruct a base tree... <stdin>:3143: new blank line at EOF. + warning: 1 line adds whitespace errors. Falling back to patching base and 3-way merge... Auto-merging opac/opac-user.pl Auto-merging members/moremember.pl Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Auto-merging installer/data/mysql/userpermissions.sql Auto-merging installer/data/mysql/userflags.sql Auto-merging installer/data/mysql/kohastructure.sql CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql Auto-merging circ/circulation.pl Auto-merging Koha/Template/Plugin/Koha.pm Auto-merging Koha/Template/Plugin/Borrowers.pm CONFLICT (content): Merge conflict in Koha/Template/Plugin/Borrowers.pm Auto-merging Koha/Schema/Result/Borrower.pm CONFLICT (content): Merge conflict in Koha/Schema/Result/Borrower.pm Auto-merging C4/Members.pm CONFLICT (content): Merge conflict in C4/Members.pm Auto-merging C4/Auth.pm Failed to merge in the changes. Patch failed at 0001 Bug 12461 - Add patron clubs feature When you have resolved this problem run git bz apply --continue. If you would prefer to skip this patch, instead run git bz apply --skip. To restore the original branch and stop patching run git bz apply --abort. Bug 12461 - Add patron clubs feature 43428 - Bug 12461 - Add patron clubs feature Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-12461---Add-patron-clubs-feature-1dbTgL.patch . Jen DeMuth
(In reply to Kyle M Hall from comment #18) > Updated patch merges constraints into table creation, fixes merge conflicts. > Tested with fresh install and upgrade! Hi, testing again. There are small conflicts. I fixed that and also have 2 follow-ups, one for koha-qa and the other to fix atomicupdate (you missed new permissions there) But I'm stuck with the test plan. Don't know how to do 7 and 9-13. I can't see any place to enroll or enable a club for OPAC. Could you explain?
Created attachment 47821 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
Created attachment 47824 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
(In reply to Bernardo Gonzalez Kriegel from comment #20) > (In reply to Kyle M Hall from comment #18) > > Updated patch merges constraints into table creation, fixes merge conflicts. > > Tested with fresh install and upgrade! > > Hi, testing again. > There are small conflicts. I fixed that and also have 2 follow-ups, > one for koha-qa and the other to fix atomicupdate (you missed > new permissions there) > > But I'm stuck with the test plan. > Don't know how to do 7 and 9-13. I can't see any place to enroll or enable a > club for OPAC. > > Could you explain? There was a bug that was preventing the Clubs tab from displaying on the opac side. Fixed!
Created attachment 47825 [details] Problem with authorized values (in staff client) With an authorized value defined as YES_NO, I get a strnge drop down when I want to enroll a patron in staff client. (Drop down list appears OK in OPAC).
Created attachment 47856 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
(In reply to Marc Véron from comment #24) > Created attachment 47825 [details] > Problem with authorized values (in staff client) > > With an authorized value defined as YES_NO, I get a strnge drop down when I > want to enroll a patron in staff client. (Drop down list appears OK in OPAC). Good catch! Should be fixed in this latest patch!
Went through test plan, works as expected. However QA tools complain: FAIL installer/data/mysql/kohastructure.sql FAIL charset_collate The table club_fields does not have the current charset collate (see bug 11944) The table club_template_enrollment_fields does not have the current charset collate (see bug 11944) The table club_enrollments does not have the current charset collate (see bug 11944) The table club_enrollment_fields does not have the current charset collate (see bug 11944) The table club_template_fields does not have the current charset collate (see bug 11944) The table club_templates does not have the current charset collate (see bug 11944) The table clubs does not have the current charset collate (see bug 11944)
Created attachment 49110 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface
All QA script issues should be fixed with this latest patch!
Created attachment 49121 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Comment on attachment 49121 [details] [review] Bug 12461 - Add patron clubs feature Review of attachment 49121 [details] [review]: ----------------------------------------------------------------- Did a first code review - not a lot of findings and mostly minor. Haven't tested the code yet. Patch doesn't apply atm - not sure why the modules appear in the general syspref.pm? <<<<<<< HEAD use C4::Log; ======= use Koha::Clubs; use Koha::Club::Enrollments; >>>>>>> Bug 12461 - Add patron clubs feature ::: Koha/Template/Plugin/AuthorisedValues.pm @@ +30,4 @@ > > sub Get { > my ( $self, $category, $selected, $opac ) = @_; > + warn "GET( $self, $category, $selected, $opac )"; Unconditional warn - please remove. ::: koha-tmpl/intranet-tmpl/prog/en/modules/clubs/clubs-add-modify.tt @@ +32,5 @@ > +<div class="yui-t7"> > + <div class="yui-main"> > + [% IF stored %] > + <div class="alert"> > + <p>Your club was [% IF stored == 'updated' %] updated [% ELSE %] saved [% END %]</p> Please avoid constructing sentences like that - it's hard to translate, because grammar and work order is different in other languages. ::: koha-tmpl/intranet-tmpl/prog/en/modules/clubs/clubs.tt @@ +23,5 @@ > + } )); > + }); > + > + function ConfirmDeleteTemplate( id, name, a ) { > + if ( confirm( _("Are you sure you want to delete the club template") + name + "?" ) ) { It would be nicer for translations to use the new Javascript syntax with placeholders here. @@ +41,5 @@ > + } > + } > + > + function ConfirmDeleteClub( id, name, a ) { > + if ( confirm( _("Are you sure you want to delete the club ") + name + "?" ) ) { Same as above - placeholders preferrable! ::: koha-tmpl/intranet-tmpl/prog/en/modules/clubs/templates-add-modify.tt @@ +9,5 @@ > +//<![CDATA[ > + > +function CheckForm() { > + if ( !$("#club-template-name").val() ) { > + alert( _("Name is a required field!") ); Would it be possible to use the standard markup for required fields insted here? See administration pages for examples. @@ +30,5 @@ > +<div class="yui-t7"> > + <div class="yui-main"> > + [% IF stored %] > + <div class="alert"> > + <p>Your club template was [% IF stored == 'updated' %] updated [% ELSE %] saved [% END %]</p> Please don't construct sentences (see above). ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/clubs/enroll.tt @@ +27,5 @@ > + </li> > + [% END %] > + > + <li> > + <a href="#" class="btn" onclick="addEnrollment(); return false;"><i class="icon-plus"></i> Enroll</a> Switch to FA icon - I saw most of them were already, maybe missed this one?
Some first tests: 1) New permissions need to be added to the permissions.inc file for translation. 2) Datatables on the new tools page are broken. 3) Misses tools sidebar navigation. I feel like the templates look a bit odd - maybe it would be better to have the different parts in separate boxes? Once this is progressed a bit further, we can worry about prettying thing up :)
Created attachment 50422 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 50423 [details] [review] Bug 12461 [QA Followup] - Changes based on suggestions
Created attachment 50426 [details] [review] Bug 12461 [QA Followup] - Changes based on suggestions
Created attachment 50427 [details] [review] Bug 12461 [QA Followup] - Changes based on suggestions
(In reply to Katrin Fischer from comment #32) > Some first tests: > 1) New permissions need to be added to the permissions.inc file for > translation. Fixed! > 2) Datatables on the new tools page are broken. Fixed! > 3) Misses tools sidebar navigation. Fixed! > I feel like the templates look a bit odd - maybe it would be better to have > the different parts in separate boxes? Once this is progressed a bit > further, we can worry about prettying thing up :) I agree. The presentation of this feature requires layouts that have not been used in Koha before. However, this followup goes a very long way to putting elements into a more "Koha-like" display. I think you'll find all your concerns have been addressed!
I am sorry, there are too many conflicts for me to feel comfortable trying to fix: CONFLICT (content): Merge conflict in opac/opac-user.pl Auto-merging members/moremember.pl Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Auto-merging installer/data/mysql/kohastructure.sql CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql Auto-merging circ/circulation.pl CONFLICT (content): Merge conflict in circ/circulation.pl Auto-merging Koha/Schema/Result/Borrower.pm Auto-merging Koha/Patron.pm CONFLICT (content): Merge conflict in Koha/Patron.pm Please take a look!
Kyle, we would like to get the academy students testing this again, but the patches do not apply. If you have a chance to rebase before Monday please do, otherwise please remove the Academy tag
Created attachment 58818 [details] [review] Bug 12461 [QA Followup] - Changes based on suggestions
Created attachment 58819 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 58820 [details] [review] Bug 12461 [QA Followup] - Changes based on suggestions
(In reply to Chris Cormack from comment #39) > Kyle, we would like to get the academy students testing this again, but the > patches do not apply. If you have a chance to rebase before Monday please > do, otherwise please remove the Academy tag Done!
There are a large amount of conflicts when applying patch which makes it rather difficult to test. If these could be fixed that would help. Cheers.
Created attachment 59272 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 59273 [details] [review] Bug 12461 [QA Followup] - Changes based on suggestions
You have left in the '>>>>>>> BUG XXXXX' when resolving the conflicts in Koha/Patron.pm (line 644) and installer/data/mysql/kohastructure.sql (lines 4077 and 4078). I cannot work out how to add a patron to a club however. If you could walk me through the process that would be awesome. Also when adding another field in the creation of a template there is no colon after the field name unlike the default fields when editing the club. There is also an extra full stop in the tools homepage under the 'Patron clubs' link. Looks like it will be a great feature when finished!
Kyle, could you please provide followup due to bug 16239? See https://wiki.koha-community.org/wiki/Interface_patterns
(In reply to Josef Moravec from comment #48) > Kyle, could you please provide followup due to bug 16239? > > See https://wiki.koha-community.org/wiki/Interface_patterns Josef, can you please be more specific? Thanks!
(In reply to Kyle M Hall from comment #49) > (In reply to Josef Moravec from comment #48) > > Kyle, could you please provide followup due to bug 16239? > > > > See https://wiki.koha-community.org/wiki/Interface_patterns > > Josef, can you please be more specific? > > Thanks! Of course, you need to use new btn classes due to Bootstrap 3 - "btn" is not enough, you need to change it to "btn btn-default" also btn size classes were renamed: "btn-mini" -> "btn-xs" "btn-small" -> "btn-sm"
Thanks! That helps a lot! (In reply to Josef Moravec from comment #50) > (In reply to Kyle M Hall from comment #49) > > (In reply to Josef Moravec from comment #48) > > > Kyle, could you please provide followup due to bug 16239? > > > > > > See https://wiki.koha-community.org/wiki/Interface_patterns > > > > Josef, can you please be more specific? > > > > Thanks! > > Of course, you need to use new btn classes due to Bootstrap 3 - "btn" is not > enough, you need to change it to "btn btn-default" also btn size classes > were renamed: > "btn-mini" -> "btn-xs" > "btn-small" -> "btn-sm"
Created attachment 60519 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 60520 [details] [review] Bug 12461 [QA Followup] - Changes based on suggestions
Created attachment 60521 [details] [review] Bug 12461 [QA Followup] - Changes based on Koha standard interface pattern
In my opinion, this patchset is not ready to be QAed. Lot of changes should be done to meet our coding guidelines.
(In reply to Jonathan Druart from comment #55) > In my opinion, this patchset is not ready to be QAed. Lot of changes should > be done to meet our coding guidelines. Jonathan, can you be more specific? That sounds like QA to me ; ) I've been working on the feature since 2012. It's already gone through one full rewrite because Koha's coding guidelines had moved on so much since it was initially written. If you want another signoff, that's fine. But you specifically wrote "Lot of changes should be done to meet our coding guidelines" which is not a task for a signer. That's a task for a QA'er and I'll really need some actionable comments if I am to make any changes! If you want another sign-off, please set status to needs sign-off. Otherwise the status can go back to failed qa.
Kyle, I did not write the different failures because most of them are obvious: 1. There are no tests 2. CamelCase instead of snake_case for methods of Koha::Clubs and Koha::Patron 3. GetClubEnrollmentsCount and GetEnrollableClubsCount are not needed, just call ->count 4. You should not need to add GetBranches to the Branches TT plugin. Take a look at options_for_libraries 5. Subroutine added to Koha TT plugin (UserEnv), but not used. 6. Changes to circ/circulation.pl are not needed. A variable 'patron' is already passed to the template. 7. All the labels of the form has the same value for the "for" attribute. 8. Tables on clubs/clubs.pl are ugly (missing datatables.css?) 9. On /clubs/clubs-add-modify.pl?club_template_id=1, there is an empty input at the end of the form. 8. CGI->param must not be called in scalar context 9. When I save a form, I get "Your club was created" with a link to "Return to patron clubs". That sounds weird. Especially because the box is yellow. I'd suggest to behave as everywhere else in Koha: Save and get the list. I guess it's because there are too many small scripts, maybe it would have been easier to have a bigger one and have the different action handled in it (as everywhere else in admin scripts). 10. INSERT statements should have IGNORE in update db process 11. + `branchcode` varchar(10) COLLATE utf8_unicode_ci NULL DEFAULT NULL, you should not need to specify the COLLATE here. 12. QA tests do not pass FAIL Koha/Patron.pm FAIL forbidden patterns forbidden pattern: merge marker (>>>>>>>) (line 647)
When I hit /circ/circulation.pl?borrowernumber=42, I get a software error: Template process failed: undef error - The method category_type is not covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121. Yes I think another signoff is needed.
Thanks! That helps a lot! (In reply to Jonathan Druart from comment #57) > Kyle, > > I did not write the different failures because most of them are obvious: > > 1. There are no tests > 2. CamelCase instead of snake_case for methods of Koha::Clubs and > Koha::Patron > 3. GetClubEnrollmentsCount and GetEnrollableClubsCount are not needed, just > call ->count > 4. You should not need to add GetBranches to the Branches TT plugin. Take a > look at options_for_libraries > 5. Subroutine added to Koha TT plugin (UserEnv), but not used. > 6. Changes to circ/circulation.pl are not needed. A variable 'patron' is > already passed to the template. > 7. All the labels of the form has the same value for the "for" attribute. > 8. Tables on clubs/clubs.pl are ugly (missing datatables.css?) > 9. On /clubs/clubs-add-modify.pl?club_template_id=1, there is an empty input > at the end of the form. > 8. CGI->param must not be called in scalar context > 9. When I save a form, I get "Your club was created" with a link to "Return > to patron clubs". That sounds weird. Especially because the box is yellow. > I'd suggest to behave as everywhere else in Koha: Save and get the list. > I guess it's because there are too many small scripts, maybe it would have > been easier to have a bigger one and have the different action handled in it > (as everywhere else in admin scripts). > 10. INSERT statements should have IGNORE in update db process > 11. + `branchcode` varchar(10) COLLATE utf8_unicode_ci NULL DEFAULT NULL, > you should not need to specify the COLLATE here. > 12. QA tests do not pass > FAIL Koha/Patron.pm > FAIL forbidden patterns > forbidden pattern: merge marker (>>>>>>>) (line 647)
Created attachment 60610 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 60611 [details] [review] Bug 12461 - Use snake case
Created attachment 60612 [details] [review] Bug 12461 - Revert changes to circulation.pl
Created attachment 60613 [details] [review] Bug 12461 - Template updates
Created attachment 60614 [details] [review] Bug 12461 - Fix 'for=' and param issues
Created attachment 60615 [details] [review] Bug 12461 - Return to clubs home after save
Created attachment 60616 [details] [review] Bug 12461 - Remote collates from branchcode rows, add IGNORE for inserts
Created attachment 60617 [details] [review] Bug 12461 - Add patron clubs feature
Still needs unit tests.
Created attachment 60618 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 60624 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Did you mean to push this to master?
Changed to critical for the feature it blocks, but this is not a blocker anymore.
Doh wrong bug.
(In reply to Liz Rea from comment #71) > Did you mean to push this to master? It has been pushed and reverted...
(In reply to Jonathan Druart from comment #74) > (In reply to Liz Rea from comment #71) > > Did you mean to push this to master? > > It has been pushed and reverted... Yes, my bad. Accidentally applied a followup to my working branch instead of master so it went along for the ride! As much as I'd love for this to be in master it's not quite ready ; )
a/clubs/clubs-add-modify.pl 47 my $stored = $cgi->param('name') ? $id ? 'updated' : 'stored' : undef; *This is confusing a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt 85 This item is on hold for another borrower. *Should be patron Koha/Template/Plugin/AuthorisedValues.pm 38 my ( $self, $category, $selected, $opac ) = @_; 39 return GetAuthorisedValues( $category, $selected, $opac ); *I think this is rebase error, no 'selected' param now koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt: [% IF borrower.get_club_enrollments_count.size || borrower.get_enrollable_clubs(1).size %] *causes error - The method size is not covered by tests *same as above in: koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt koha-tmpl/opac-tmpl/bootstrap/en/modules/clubs/clubs-tab.tt * borrower.FirstValidEmailAddress is not covered by tests, I think you mean borrower.first_valid_email_address When viewing tools/clubs.pl 1 - The 'New club' button has an empty dropdown until a template is defined, it should be disabled 2 - Once the first template is defined the Clubs table says: " No club templates defined. " - should say "No clubs defined" 3 - Dates for clubs should show in clubs table When adding patrons to clubs - 1 - If no fields are associated with a club you click 'Enroll' then have a new screen with no additional info and must click 'Enroll' again - should be clearer 'Finish enrollment' or something 2 - You should not be able to add a patron to club that is out of date, or they should be hidden by default The feature adds valuable info for reporting, but I think it should have at least one default view, a link from the clubs table to "See patrons enrolled in this club" That link should be copied on the patrons club enrollement tab
Created attachment 62778 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 62779 [details] [review] Bug 12461 [QA Followup]
Created attachment 62813 [details] [review] Bug 12461 - Add patron clubs feature This features would add the ability to create clubs which patrons may be enrolled in. It would be particularly useful for tracking summer reading programs, book clubs and other such clubs. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Ensure your staff user has the new 'Patron clubs' permissions 4) Under the tools menu, click the "Patron clubs" link 5) Create a new club template * Here you can add fields that can be filled out at the time a new club is created based on the template, or a new enrollment is created for a given club based on the template. 6) Create a new club based on that template 7) Attempt to enroll a patron in that club 8) Create a club with email required set 9) Attempt to enroll a patron without an email address in that club 10) Create a club that is enrollable from the OPAC 11) Attempt to enroll a patron in that club 12) Attempt to cancel a club enrollment from the OPAC 13) Attempt to cancel a club enrollment from the staff interface Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 62814 [details] [review] Bug 12461 [QA Followup] Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 62815 [details] [review] Bug 12461 - (QA Followup) Club enrollments should only list active enrollments Confirms should warn of cascading delete Branch limited clubs should only be available to members of that branch Fix breadcrumbs
Pushed to master for 17.05, thanks Nick!
I do not understand the $return_scalar argument you use for get_club_enrollments and get_enrollable_clubs. Could you please explain? It seems useless, I suggest to remove it.
(In reply to Jonathan Druart from comment #83) > I do not understand the $return_scalar argument you use for > get_club_enrollments and get_enrollable_clubs. Could you please explain? > > It seems useless, I suggest to remove it. It is a hack solution to an big issue with template toolkit. Basically, when calling a method from Template Toolkit that would return a Koha::Objects derived class, the result will always be returned in list context. I've tried using the scalar plugin for TT but had no luck. Even worse, if the list has only 1 element, TT will convert it to just being that element with no list context. Then, if you try to call a TT list method ( such as 'size' ), it will try to call that method on the object instead and you'll get an error!
This won't get ported back to 16.11.x as it is a new feature.