Description
Jonathan Druart
2017-12-14 21:18:34 UTC
*** Bug 19381 has been marked as a duplicate of this bug. *** Created attachment 69807 [details] [review] Bug 19817: Redirect to the online manual Created attachment 69808 [details] [review] Bug 19817: Remove local help files + edit help feature Created attachment 70131 [details] [review] Bug 19817: Make the location of the manual configurable - KohaManualBaseURL Created attachment 70132 [details] [review] Bug 19817: Move code to Koha::Manual to make it testable and reusable Created attachment 70133 [details] [review] Bug 19817: Add new permission edit_help Change in existing behaviour! Next step is to re-add the ability to "edit the manual". I would suggest to remove the possibility to edit the manual, but allow to create new pages (which will not be erased on update). I need to know what are the different use cases and needs. We need also to think about an ergonomic way to manage these additional help files. For instance: in the top right of the staff pages we will have a "Help" group buttons with Link to the official manual | Add a new page And when new pages are created: Link to the official manual | Additional pages 1..N | Add a new page The additional pages would be linked to a specific page of the staff interface (see the mapping in Koha::Manual) Does it make sense? Other ideas/opinions? (In reply to Jonathan Druart from comment #7) > Next step is to re-add the ability to "edit the manual". > I would suggest to remove the possibility to edit the manual, but allow to > create new pages (which will not be erased on update). > > I need to know what are the different use cases and needs. Possible use cases I could see for local help files: - Being able to explain custom fields like authorised values, patron attributes etc. - Being able to explain how to use collections, locations, item types - Offer local help for cataloguing like explanations for fields and subfields, internal standards and agreements > We need also to think about an ergonomic way to manage these additional help > files. > For instance: in the top right of the staff pages we will have a "Help" > group buttons with > Link to the official manual | Add a new page > And when new pages are created: > Link to the official manual | Additional pages 1..N | Add a new page > > The additional pages would be linked to a specific page of the staff > interface (see the mapping in Koha::Manual) > > Does it make sense? Other ideas/opinions? That makes sense to me. I think having a HTML based editor would be better than .rst for the local help files and that we should store them in the database. Hi Jonathan, can you please rebase? I tried but it got a bit chaotic. I think you are missing the change to koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc for the new permission. Created attachment 73523 [details] [review] Bug 19817: Redirect to the online manual Created attachment 73524 [details] [review] Bug 19817: Remove local help files + edit help feature Created attachment 73525 [details] [review] Bug 19817: Make the location of the manual configurable - KohaManualBaseURL Created attachment 73526 [details] [review] Bug 19817: Move code to Koha::Manual to make it testable and reusable Created attachment 73527 [details] [review] Bug 19817: Add new permission edit_help Change in existing behaviour! Done, note that the last patch does nothing (new permission is not used). Hi Jonathan, some thoughts: 1) sub _get_base_url doesn't look quite right yet. a) It hardcodes en. Ideally we should try to check for the existence of a translated manual and use that, with a fallback to en. If we can't do that, we'll maybe need to add a pref where you can add the lanugage codes to use. b) For the local manual URL you combine: C4::Context->preference('staffClientBaseURL') . $KohaManualBaseURL; I think it might be good to allow for a full URL in KohaManualBaseURL. That way you could have the help on the same server or at another location. 2) If the permission doesn't work yet, I would not include it :) 3) The manual layout doesn't work well in the small pop up. I wonder if there is a way to leave off the side navigation, but probably we can just make the window a bit wider. The manual should first try to be in the currently used UI language, and then whatever other fallbacks there are (with "en" as the last, hardcoded one) or if fallbacks aren't feasible, the non-existing-manual page on koha-community could show something like "The manual does not exist for the language $foo. You can read the [manual in english], or [translate the manual]", with links to the actual manual in english and to the wiki article on how to start translation. Created attachment 73651 [details] [review] Bug 19817: Add pref KohaManualLanguage Created attachment 73652 [details] [review] Bug 19817: Open the manual in a new window, not a popup (In reply to Katrin Fischer from comment #16) > Hi Jonathan, > > some thoughts: > > 1) sub _get_base_url doesn't look quite right yet. > a) It hardcodes en. Ideally we should try to check for the existence of a > translated manual and use that, with a fallback to en. If we can't do that, > we'll maybe need to add a pref where you can add the lanugage codes to use. I have added a patch to add a new pref KohaManualLanguage > b) For the local manual URL you combine: > C4::Context->preference('staffClientBaseURL') . $KohaManualBaseURL; > I think it might be good to allow for a full URL in KohaManualBaseURL. That > way you could have the help on the same server or at another location. The concatenation is done only if the pref does not start with '/', so you can use a value starting with 'http://' > 2) If the permission doesn't work yet, I would not include it :) Patch removed. > 3) The manual layout doesn't work well in the small pop up. I wonder if > there is a way to leave off the side navigation, but probably we can just > make the window a bit wider. Done. I think the pref is too limiting for a lot of multilingual instances still. Could we make it work like that? Make the manual pref a list of language codes to existing manuals. Try to open the manual in the current GUI language first using the list to verify if it exists. If it doesn't match one of the language codes, use the first code in the list as a fallback. We could start the list off with all the existing manuals and first language en, but it would be easy to add new ones. Created attachment 73872 [details] [review] Bug 19817: Use the language from the interface if valid Use the UI language for the manual, if exists. Use the pref as a fallback. With the call to get_template_and_user the value of preferred_language switched from 'es' (spanish) to 'en' from one click to another (??) Created attachment 73877 [details] [review] Bug 19817: Use the language from the interface if valid Use the UI language for the manual, if exists. Use the pref as a fallback. With the call to get_template_and_user the value of preferred_language switched from 'es' (spanish) to 'en' from one click to another (??) Comment on attachment 73523 [details] [review] Bug 19817: Redirect to the online manual Review of attachment 73523 [details] [review]: ----------------------------------------------------------------- ::: help.pl @@ +1,3 @@ > #!/usr/bin/perl > > +# Copyright 2017 Koha Development team I would prefer 2010-2017 or even 2010,2017 so we do not lose track of the original date in quickly obvious statement. (https://www.plagiarismtoday.com/2011/11/08/how-to-write-a-copyright-notice/) Comment on attachment 73525 [details] [review] Bug 19817: Make the location of the manual configurable - KohaManualBaseURL Review of attachment 73525 [details] [review]: ----------------------------------------------------------------- The http URL redirects to an https one, so it makes more sense to use https immediately. ::: help.pl @@ +39,2 @@ > # FIXME /en/ must be configurable (or guessed) > +my $KohaManualBaseURL = C4::Context->preference('KohaManualBaseURL') || 'http://koha-community.org/manual'; please use https. ::: installer/data/mysql/atomicupdate/bug_19817.sql @@ +1,2 @@ > +INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) > +VALUES ('KohaManualBaseURL','http://koha-community.org/manual/','','Where is the Koha manual/documentation located?','Free'); please use https. ::: installer/data/mysql/sysprefs.sql @@ +238,4 @@ > ('itemBarcodeInputFilter','','whitespace|T-prefix|cuecat|libsuite8|EAN13','If set, allows specification of a item barcode input filter','Choice'), > ('itemcallnumber','082ab',NULL,'The MARC field/subfield that is used to calculate the itemcallnumber (Dewey would be 082ab or 092ab; LOC would be 050ab or 090ab) could be 852hi from an item record','free'), > ('KohaAdminEmailAddress','root@localhost','','Define the email address where patron modification requests are sent','free'), > +('KohaManualBaseURL','http://koha-community.org/manual/','','Where is the Koha manual/documentation located?','Free'), please use https. ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/enhanced_content.pref @@ +394,5 @@ > + Manual: > + - > + - Location of the Koha manual > + - pref: KohaManualBaseURL > + - You can specify the location of the Koha manual to use. By default it is http://koha-community.org/manual/ please use https. Comment on attachment 73526 [details] [review] Bug 19817: Move code to Koha::Manual to make it testable and reusable Review of attachment 73526 [details] [review]: ----------------------------------------------------------------- I may have missed commenting on other changes which had just http in them. ::: Koha/Manual.pm @@ +17,5 @@ > +} > + > +sub _get_base_url { > + # FIXME /en/ must be configurable (or guessed) > + my $KohaManualBaseURL = C4::Context->preference('KohaManualBaseURL') || 'http://koha-community.org/manual'; Please use https. Comment on attachment 73651 [details] [review] Bug 19817: Add pref KohaManualLanguage Review of attachment 73651 [details] [review]: ----------------------------------------------------------------- ::: Koha/Manual.pm @@ +20,2 @@ > my $KohaManualBaseURL = C4::Context->preference('KohaManualBaseURL') || 'http://koha-community.org/manual'; > + my $KohaManualLanguage = C4::Context->preference('KohaManualLanguage') || 'en'; If there are multiple languages selected, this will be ugly and broken, I think (haven't tested yet). ::: installer/data/mysql/sysprefs.sql @@ +47,4 @@ > ('AnonSuggestions','0',NULL,'Set to enable Anonymous suggestions to AnonymousPatron borrowernumber','YesNo'), > ('AnonymousPatron','0',NULL,'Set the identifier (borrowernumber) of the anonymous patron. Used for Suggestion and reading history privacy',''), > ('ArticleRequests', '0', NULL, 'Enables the article request feature', 'YesNo'), > +('ArticleRequestsMandatoryFields', '', NULL, 'Comma delimited list of required fields for bibs where article requests rule = ''yes''', 'Choice'), Was this another bug fix someewhere? multiple->Choice and removal of end of line whitespace. @@ +239,4 @@ > ('itemcallnumber','082ab',NULL,'The MARC field/subfield that is used to calculate the itemcallnumber (Dewey would be 082ab or 092ab; LOC would be 050ab or 090ab) could be 852hi from an item record','free'), > ('KohaAdminEmailAddress','root@localhost','','Define the email address where patron modification requests are sent','free'), > ('KohaManualBaseURL','http://koha-community.org/manual/','','Where is the Koha manual/documentation located?','Free'), > +('KohaManualLanguage','en','en|ar|cs|es|fr|it|pt_BR|tr|zh_TW','What is the language of the online manual you want to use?','multiple'), I like the idea of calling up the language which is currently selected, if possible. However, multiple differs from Choice used in the atomic update. Need to correct somehow. Comment on attachment 73524 [details] [review] Bug 19817: Remove local help files + edit help feature Review of attachment 73524 [details] [review]: ----------------------------------------------------------------- ::: t/db_dependent/00-strict.t @@ +19,4 @@ > 'catalogue', 'cataloguing', > 'changelanguage.pl', 'circ', > 'debian', 'docs', > + 'errors', 'fix-perl-path.PL', But help.pl is no longer listed. @@ -19,5 @@ > 'catalogue', 'cataloguing', > 'changelanguage.pl', 'circ', > 'debian', 'docs', > - 'edithelp.pl', 'errors', > - 'fix-perl-path.PL', 'help.pl', help.pl is still here. Preliminary testing was generally positive. Though, I think these points should be discussed, if not addressed, before considering sign off. - https vs http in the urls used. - choice vs multiple in the added KohaManualLanguage sys pref - help.pl removed from 00-strict.t - postfix slash the default URL creates a // in the URL. remove the '/' in the URL building in Koha::Manual? build smarter? kshell cd misc/translator for i in `ls po | cut -f1-2 -d'-' | sed -e "s#-opac##g" | sed -e "s#-pref.po##g" | sed -e "s#-staff##g" | sed -e "s#-marc##g" | sort -u`; do echo $i;perl translate install $i;done -- noisy! Though, I suspect correct. perhaps help po files need to be deleted too? Created attachment 73931 [details] [review] Bug 19817: Redirect to the online manual Created attachment 73932 [details] [review] Bug 19817: Remove local help files + edit help feature Created attachment 73933 [details] [review] Bug 19817: Make the location of the manual configurable - KohaManualBaseURL Created attachment 73934 [details] [review] Bug 19817: Move code to Koha::Manual to make it testable and reusable Created attachment 73935 [details] [review] Bug 19817: Add pref KohaManualLanguage Created attachment 73936 [details] [review] Bug 19817: Open the manual in a new window, not a popup Created attachment 73937 [details] [review] Bug 19817: Use the language from the interface if valid Use the UI language for the manual, if exists. Use the pref as a fallback. With the call to get_template_and_user the value of preferred_language switched from 'es' (spanish) to 'en' from one click to another (??) Created attachment 73938 [details] [review] Bug 19817: use https instead of http the https version was not working when I wrote the first patches (In reply to M. Tompsett from comment #29) > Preliminary testing was generally positive. Though, I think these points > should be discussed, if not addressed, before considering sign off. Thanks for catching those things. I have marked as obsolete the other comments (they are the verbose version of the next 5 lines) > - https vs http in the urls used. Yes, in the meanwhile I have asked rangi to enable https on the manual (was not there before). > - choice vs multiple in the added KohaManualLanguage sys pref It's a choice indeed. > - help.pl removed from 00-strict.t Re-added > - postfix slash the default URL creates a // in the URL. > remove the '/' in the URL building in Koha::Manual? > build smarter? I do not understand what you suggest here. > kshell > cd misc/translator > for i in `ls po | cut -f1-2 -d'-' | sed -e "s#-opac##g" | sed -e > "s#-pref.po##g" | sed -e "s#-staff##g" | sed -e "s#-marc##g" | sort -u`; do > echo $i;perl translate install $i;done > -- noisy! Though, I suspect correct. > perhaps help po files need to be deleted too? I do not think so, should be deleted on releasing (there are certainly other things to do in the release scripts). The patches don't apply right now - can you take a look? Looking at the last comments: 1) Removing the po files on release date would make sense to me. 2) >> - postfix slash the default URL creates a // in the URL. >> remove the '/' in the URL building in Koha::Manual? >> build smarter? I think we expect people to get it right for BaseURLs already. so no need to be more strict here. (Do not enter trailing slash) 3) >> - choice vs multiple in the added KohaManualLanguage sys pref >It's a choice indeed. Hm, what I was thinking about was a multiple indeed. We usually have at least German and English installed for staff, often Spanish and French now too. I would like to use the pref to indicate existing manuals for staff activated languages. So when I open the help and it matches one of the selected languages I get the right language manual. Makes sense? Created attachment 74567 [details] [review] Bug 19817: Redirect to the online manual Created attachment 74568 [details] [review] Bug 19817: Remove local help files + edit help feature Created attachment 74569 [details] [review] Bug 19817: Make the location of the manual configurable - KohaManualBaseURL Created attachment 74570 [details] [review] Bug 19817: Move code to Koha::Manual to make it testable and reusable Created attachment 74571 [details] [review] Bug 19817: Add pref KohaManualLanguage Created attachment 74572 [details] [review] Bug 19817: Open the manual in a new window, not a popup Created attachment 74573 [details] [review] Bug 19817: Use the language from the interface if valid Use the UI language for the manual, if exists. Use the pref as a fallback. With the call to get_template_and_user the value of preferred_language switched from 'es' (spanish) to 'en' from one click to another (??) Created attachment 74574 [details] [review] Bug 19817: use https instead of http the https version was not working when I wrote the first patches (In reply to Katrin Fischer from comment #39) > 3) > >> - choice vs multiple in the added KohaManualLanguage sys pref > > >It's a choice indeed. > > Hm, what I was thinking about was a multiple indeed. We usually have at > least German and English installed for staff, often Spanish and French now > too. I would like to use the pref to indicate existing manuals for staff > activated languages. So when I open the help and it matches one of the > selected languages I get the right language manual. Makes sense? The pref is the default value. If the pref is set to English and the interface is in Spanish, the manual will be in Spanish. If the interface is in Esperanto, the manual will be in English (the manual is not translated in Esperanto, yet). What if the selected language doesn't have an existing manual? My idea was to use the pref as a white list of existing manuals, because I was not sure we can tell and not have it end up on an error page. To me it does not make sense to make it a free text. We could retrieve the available languages from the syspref options, it will avoid to have it hardcoded in the code. And then a quick SQL query will allow to extend the list. I do not think it should be a blocker anyway. Created attachment 74644 [details] [review] Bug 19817: Redirect to the online manual Created attachment 74645 [details] [review] Bug 19817: Remove local help files + edit help feature Created attachment 74646 [details] [review] Bug 19817: Make the location of the manual configurable - KohaManualBaseURL Created attachment 74647 [details] [review] Bug 19817: Move code to Koha::Manual to make it testable and reusable Created attachment 74648 [details] [review] Bug 19817: Add pref KohaManualLanguage Created attachment 74649 [details] [review] Bug 19817: Open the manual in a new window, not a popup Created attachment 74650 [details] [review] Bug 19817: Use the language from the interface if valid Use the UI language for the manual, if exists. Use the pref as a fallback. With the call to get_template_and_user the value of preferred_language switched from 'es' (spanish) to 'en' from one click to another (??) Created attachment 74651 [details] [review] Bug 19817: use https instead of http the https version was not working when I wrote the first patches Created attachment 75370 [details] [review] Bug 19817: Redirect to the online manual Created attachment 75371 [details] [review] Bug 19817: Remove local help files + edit help feature Created attachment 75372 [details] [review] Bug 19817: Make the location of the manual configurable - KohaManualBaseURL Created attachment 75373 [details] [review] Bug 19817: Move code to Koha::Manual to make it testable and reusable Created attachment 75374 [details] [review] Bug 19817: Add pref KohaManualLanguage Created attachment 75375 [details] [review] Bug 19817: Open the manual in a new window, not a popup Created attachment 75376 [details] [review] Bug 19817: Use the language from the interface if valid Use the UI language for the manual, if exists. Use the pref as a fallback. With the call to get_template_and_user the value of preferred_language switched from 'es' (spanish) to 'en' from one click to another (??) Created attachment 75377 [details] [review] Bug 19817: use https instead of http the https version was not working when I wrote the first patches Rebased on top of bug 20706. Is there more of a test plan than "click the help link on a bunch of pages?" (In reply to Owen Leonard from comment #69) > Is there more of a test plan than "click the help link on a bunch of pages?" Yes, definitely. This change is more a philosophical changes, we are going to remove the local manual. To test the whole patchset you will need to play with KohaManualBaseURL and KohaManualLanguage and make sure the links are still working and redirect to the correct url. I had a bit of trouble applying the patch. I had to add member-flags.tt and then continue applying the patch. I don't know if that's normal or not. I went to Global system preferences and clicked on the tab for circulation. When I clicked help, the manual was at the beginning of the sysprefs section. I don't know if it's possible at all to point to the right sub-section? It's just that are sooooo many system preferences... My KohaManualLanguage is on Arabic by default. I'm guessing it's the first language in the list, but if I don't change it will this be the default? Should we put English by default instead of Arabic? I also don't see the edit button mentioned in comment 7. Was that implemented or just an idea for a future development? It could be a second patch. People don't currently have the ability to modify from the staff interface, so it wouldn't necessarily be a step back. More advanced users can still use their own help files. I wasn't able to test switching languages because my master is only installed in English. It's a great development! It will be much easier to translate, no need to translate the help file, just the manual. It will also be more uniform. (In reply to Caroline Cyr La Rose from comment #71) > I had a bit of trouble applying the patch. I had to add member-flags.tt and > then continue applying the patch. I don't know if that's normal or not. Yes, conflict. I am going to re-attached a rebased version of these patches. > I went to Global system preferences and clicked on the tab for circulation. > When I clicked help, the manual was at the beginning of the sysprefs > section. I don't know if it's possible at all to point to the right > sub-section? It's just that are sooooo many system preferences... It should be the same behaviour as prior to the patch. We can imagine a link close to each pref to point to the dedicated section of the manual. TODO later ;) > My KohaManualLanguage is on Arabic by default. I'm guessing it's the first > language in the list, but if I don't change it will this be the default? > Should we put English by default instead of Arabic? I guess you did not execute the updatedatabase.pl script. The default value is "en". > I also don't see the edit button mentioned in comment 7. Was that > implemented or just an idea for a future development? It could be a second > patch. People don't currently have the ability to modify from the staff > interface, so it wouldn't necessarily be a step back. More advanced users > can still use their own help files. The original plan was to provide a full help system, local editing (on top of git repository), etc. I have asked several time on the mailing lists and during IRC meetings to get feedbacks and ideas. I did not get answers and I am no longer willing to provide such enhancements. Maybe later... > I wasn't able to test switching languages because my master is only > installed in English. You can install a couple with: sudo koha-translate --install fr-CA --dev kohadev sudo koha-translate --install it-IT --dev kohadev > It's a great development! It will be much easier to translate, no need to > translate the help file, just the manual. It will also be more uniform. Created attachment 76151 [details] [review] Bug 19817: Redirect to the online manual Created attachment 76152 [details] [review] Bug 19817: Remove local help files + edit help feature Created attachment 76153 [details] [review] Bug 19817: Make the location of the manual configurable - KohaManualBaseURL Created attachment 76154 [details] [review] Bug 19817: Move code to Koha::Manual to make it testable and reusable Created attachment 76155 [details] [review] Bug 19817: Add pref KohaManualLanguage Created attachment 76156 [details] [review] Bug 19817: Open the manual in a new window, not a popup Created attachment 76157 [details] [review] Bug 19817: Use the language from the interface if valid Use the UI language for the manual, if exists. Use the pref as a fallback. With the call to get_template_and_user the value of preferred_language switched from 'es' (spanish) to 'en' from one click to another (??) Created attachment 76158 [details] [review] Bug 19817: use https instead of http the https version was not working when I wrote the first patches *** Bug 11115 has been marked as a duplicate of this bug. *** Created attachment 77791 [details] [review] Bug 19817: Redirect to the online manual Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 77792 [details] [review] Bug 19817: Remove local help files + edit help feature Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 77793 [details] [review] Bug 19817: Make the location of the manual configurable - KohaManualBaseURL Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 77794 [details] [review] Bug 19817: Move code to Koha::Manual to make it testable and reusable Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 77795 [details] [review] Bug 19817: Add pref KohaManualLanguage Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 77797 [details] [review] Bug 19817: Open the manual in a new window, not a popup Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 77798 [details] [review] Bug 19817: Use the language from the interface if valid Use the UI language for the manual, if exists. Use the pref as a fallback. With the call to get_template_and_user the value of preferred_language switched from 'es' (spanish) to 'en' from one click to another (??) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 77799 [details] [review] Bug 19817: use https instead of http the https version was not working when I wrote the first patches Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> All seems to work for me, signing off. There is something not quite right with the default URL and mappings. We have since removed the chapter numbers from the file names in Sphinx, but the patches seem to link to an outdated version of the manual: https://koha-community.org/manual/18.05/en/html/09_acquisitions.html#acquisition-searches vs correct: https://koha-community.org/manual/18.05/en/html/acquisitions.html#acquisition-searches Have to stop working on this for now, hope to be back later today. If you beat me to fixing this. I'd not be unhappy about it :) Also: t/db_dependent/00-strict.t fails for me, but because of Overdrive. How to fix? CPAN? Created attachment 77932 [details] [review] Bug 19817: Remove numbering from chapter file names See commit 8ad495abf1e42c5e73c5cb62ec32e1a54d16f8e8 Remove numbering from chapter file names from koha-manual (In reply to Katrin Fischer from comment #92) > Also: t/db_dependent/00-strict.t fails for me, but because of Overdrive. How > to fix? CPAN? I guess so, yes. But for sure you get it against master as well, it's not related with those patches. (In reply to Jonathan Druart from comment #94) > (In reply to Katrin Fischer from comment #92) > > Also: t/db_dependent/00-strict.t fails for me, but because of Overdrive. How > > to fix? CPAN? > > I guess so, yes. But for sure you get it against master as well, it's not > related with those patches. I know it's not related - but this patch set changes the test and the failure keeps me from testing that part. :) Created attachment 77942 [details] [review] Bug 19817: Redirect to the online manual Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 77943 [details] [review] Bug 19817: Remove local help files + edit help feature Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 77944 [details] [review] Bug 19817: Make the location of the manual configurable - KohaManualBaseURL Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 77945 [details] [review] Bug 19817: Move code to Koha::Manual to make it testable and reusable Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 77946 [details] [review] Bug 19817: Add pref KohaManualLanguage Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 77947 [details] [review] Bug 19817: Open the manual in a new window, not a popup Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 77948 [details] [review] Bug 19817: Use the language from the interface if valid Use the UI language for the manual, if exists. Use the pref as a fallback. With the call to get_template_and_user the value of preferred_language switched from 'es' (spanish) to 'en' from one click to another (??) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 77949 [details] [review] Bug 19817: use https instead of http the https version was not working when I wrote the first patches Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 77950 [details] [review] Bug 19817: Remove numbering from chapter file names See commit 8ad495abf1e42c5e73c5cb62ec32e1a54d16f8e8 Remove numbering from chapter file names from koha-manual Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 77951 [details] [review] Bug 19817: (QA follow-up) Add German manual and fix system preference help files - German language manuals were added since this patch was started - The structure of the manual was changed slightly, system preferences were moved into their own chapter To test: - Test help files display correctly for system preferences display correctly Note: Local use uses a different page than the other tabs - Test German shows up as option in KohaManualLanguage and links correctly Awesome work all! Pushed to master for 18.11 New feature, this will not be backported to 18.05.x series. Created attachment 78953 [details] [review] Bug 19817: (follow-up) Remove local help files + edit help feature I have no idea what happened but RM did not rebase that one correctly before pushing it ;) (In reply to Jonathan Druart from comment #108) > Created attachment 78953 [details] [review] [review] > Bug 19817: (follow-up) Remove local help files + edit help feature > > I have no idea what happened but RM did not rebase that one correctly > before pushing it ;) What happened = The RM goofed Follow-up pushed to master Setting KohaManualBaseURL to https://koha-community.org/manual and using user interface language pt~PT, the help link is redirected (for example) to https://koha-community.org/manual/18.11/pt_BZ/html/systempreferences.html displaying an error page. I guess its related with a typo on line 22 in the file Manual.pm 22 my @available_languages = qw( en ar cs es fr it pt_BZ tz zh_TW ); Tested replacing the value pt_BZ by pt_BR and the link now works. Created attachment 87238 [details] [review] Fix Portuguese manual link This should really be added as a new bug Humberto, otherwise it's extremely easy for the maintenance team to miss it. I'll split it off for you this time. See bug 22687 |