Description
Martin Persson
2015-12-07 14:53:42 UTC
Created attachment 45621 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Created attachment 58272 [details] [review] Bug 15326: [FOLLOW-UP] Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Applying: Bug 15326: Add CMS feature fatal: sha1 information is lacking or useless (Koha/CmsPage.pm). We seem to miss patches here? Tried it on a freshly installed kohadevbox with jessie with the same result as Marcel. Created attachment 58607 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Created attachment 58608 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Created attachment 58609 [details] [review] Bug 15326: [FOLLOW-UP] Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland The patch was not applying because I accidentally obsoleted Martin's initial patch, which is required! I have fixed any merge conflicts. This should apply cleanly and is now ready to test. Created attachment 58787 [details] [review] Bug 15326: [FOLLOW-UP] Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Applying: Bug 15326: [FOLLOW-UP] Add CMS feature fatal: sha1 information is lacking or useless (Koha/CmsPages.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Created attachment 63904 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Created attachment 63905 [details] [review] Bug 15326: [FOLLOW-UP] Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Merge conflicts fixed, ready for testing It basically works, but need more improvements: - the action buttons should be in dropdown and contain also delete button - both test fail - there are also many issues from koha qa test tool: FAIL Koha/CmsPages.pm FAIL critic "return" statement with explicit "undef" at line 93, column 17. See page 199 of PBP. "return" statement with explicit "undef" at line 80, column 13. See page 199 of PBP. FAIL pod *** ERROR: Apparent command =cut not preceded by blank line in file Koha/CmsPages.pm FAIL spelling paramters ==> parameters FAIL koha-tmpl/intranet-tmpl/prog/en/modules/tools/cmspages.tt FAIL forbidden patterns forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 284) forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 286) forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 288) forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 281) forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 283) forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 92) FAIL t/db_dependent/Pages.t FAIL forbidden patterns forbidden pattern: tab char (line 78) forbidden pattern: tab char (line 49) forbidden pattern: tab char (line 85) forbidden pattern: tab char (line 54) forbidden pattern: tab char (line 72) forbidden pattern: tab char (line 84) forbidden pattern: tab char (line 62) forbidden pattern: tab char (line 82) forbidden pattern: tab char (line 68) forbidden pattern: tab char (line 70) forbidden pattern: tab char (line 73) forbidden pattern: tab char (line 50) forbidden pattern: tab char (line 59) forbidden pattern: tab char (line 52) forbidden pattern: tab char (line 71) forbidden pattern: tab char (line 79) forbidden pattern: tab char (line 61) forbidden pattern: tab char (line 75) forbidden pattern: tab char (line 58) forbidden pattern: tab char (line 60) forbidden pattern: tab char (line 63) forbidden pattern: tab char (line 48) forbidden pattern: tab char (line 64) forbidden pattern: tab char (line 53) forbidden pattern: tab char (line 55) forbidden pattern: tab char (line 83) forbidden pattern: tab char (line 74) forbidden pattern: tab char (line 81) forbidden pattern: tab char (line 80) forbidden pattern: tab char (line 69) forbidden pattern: tab char (line 65) forbidden pattern: tab char (line 51) Created attachment 64241 [details] [review] Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Not ok yet, sorry: Test t/Pages.t: There is no C4::Content module in Koha... # Failed test 'use C4::Content;' # at t/Pages.t line 9. # Tried to use 'C4::Content'. # Error: Can't locate C4/Content.pm in @INC (you may need to install the C4::Content module) (@INC contains: /home/vagrant/kohaclone /home/vagrant/qa-test-tools /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.20.2 /usr/local/share/perl/5.20.2 /usr/lib/x86_64-linux-gnu/perl5/5.20 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.20 /usr/share/perl/5.20 /usr/local/lib/site_perl .) at t/Pages.t line 9. # BEGIN failed--compilation aborted at t/Pages.t line 9. Undefined subroutine &C4::Content::_SQLToPerl called at t/Pages.t line 30. # Looks like your test exited with 255 just after 1. also, when using this tool - adding and editing does not work after last patch - some values are not saved (publish status), some are not populated to form when editing (library)... when I set page for all libraries, i get ISE with this log: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`cms_pages`, CONSTRAINT `cmspages_branchcode_ibfk` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "INSERT INTO `cms_pages` ( `branchcode`, `content`, `id`, `lang`, `location`, `parent`, `publish`, `sortorder`, `title`, `title_link`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0="", 1="<p>asdasdasd</p>", 2="", 3="", 4="", 5="", 6="on", 7="10", 8="asdasd", 9="asdfasd"] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. DBIx::Class::Storage::DBI::_dbh_execute(): Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`cms_pages`, CONSTRAINT `cmspages_branchcode_ibfk` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE) at /home/vagrant/kohaclone/Koha/CmsPages.pm line 94 Created attachment 64941 [details] [review] Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Created attachment 64942 [details] [review] Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. The library field is still not populated corectly to the form when editing existing page... No page is accesible via opac now - everypage just says "This page has not been published", even if I have it published for all librarias, interfaces and languages... Created attachment 66255 [details] [review] Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Hi, this is a nice feature.. just a couple of questions a super user always is able to see all pages right? at least this is what it happens to us. be aware of children pages should have a greater order than parents if not, they will be published as children of other links... for example we have 3 pages 2 root and one children... if the order of children is lower than parent. it appears as children of the other like this root1 . children_of_root_2 root2 I can't see where pages are published in staff. in opac we see them in opacnav The links are not published in opac account page, right? Great job so far! Things I didn't find or that I'm not sure about (please note that I am still new to Koha, so there might be some things I'm just missing because of inexperience): - I didn't find the pages in the staff interface (I found the tool to create the pages, but from the tool I could see all the pages, even the ones restricted to OPAC) - If your OPAC and staff interface are in different languages, do you have to create all the pages in double/triple/quadruple (one limited to the first language, the second limited to the second language, etc.)? - in OPAC, am I supposed to see the pages for all sites, or just the ones of my home library (if I'm connected)? Right now, even though I'm connected, I see all pages. Things that work A1!: - I don't see the "staff only pages" in the OPAC - The WYSIWYG interface is good. I didn't test everything, but headers, bold and lists work as they usually do. Things to work on : - The links for the pages don't appear on the main OPAC page unless I put something (anything) in the OpacNav system preference. I put some text there, no html, and the list of pages appeared. Otherwise, it was invisible. - Also, on the home page, the links seem to appear in the following order: first, all the parent pages in the order they were created, next, the child pages in the order they were created (not under their parent, but at the end of the list). After I click on one of the links, the links rearrange themselves so that the child pages are under their parent. The pages still appear in the order they were created even though I put different values in the "Appear in position" field. - When I click on a parent page, the child pages dissapear from the navigation list - When there's an error when creating a page, the page is just deleted, so you lose all the work you just did instead of being able to modify the error. For example, on the second page I created, I got an error message (I think it was that the link wasn't unique) and was directed back to the list of pages (and lost all the text I had put in the contents field), instead of the link field being highlighted so I could just change it so that it was unique and resave the page. Created attachment 70866 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Created attachment 70867 [details] [review] Bug 15326: [FOLLOW-UP] Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Created attachment 70868 [details] [review] Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Created attachment 70869 [details] [review] Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Created attachment 70870 [details] [review] Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. Created attachment 70871 [details] [review] Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Created attachment 76387 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 76388 [details] [review] Bug 15326: (follow-up) Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 76389 [details] [review] Bug 15326: (follow-up) utf8 to utf8mb4 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 76390 [details] [review] Bug 15326: (follow-up) Move js to footer, use Asset plugin Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 76391 [details] [review] Bug 15326: (follow-up) Don't make links to unpublished pages in opac Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 76392 [details] [review] Bug 15326: (follow-up) Fix tools menu highlighting Signed-off-by: Josef Moravec <josef.moravec@gmail.com> I squashed the original patches for better readability, but then added 4 new follow-ups ;) I tried to reflect the last changes in codebase and coding guidelines. (things are changing quickly). Note for QA: the feature is working OK, but there are definitely some areas where the code could be improved. Trying to tackle this: 1) QA scripts Only the second looks valid, as this class is new and added by the patch set: FAIL Koha/CmsPage.pm FAIL pod coverage POD coverage was greater before, try perl FAIL Koha/CmsPages.pm FAIL pod coverage POD is missing for 'object_class' FAIL Koha/Schema/Result/CmsPage.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 Can you please fix? 2) Code review a) Schema patches would be much better in a separate patch. Ok for now as the patch still applies. For a rebase separating those change might be good. b) In opac-cmspages there is: + flagsrequired => { borrow => 1 }, The borrow permission has been removed since. (blocker) c) Comment was removed unnecessarily, just needs to be moved into its right spot below again: -# If GoogleIndicTransliteration system preference is On Set parameter to load Google's javascript in OPAC search screens d) One file uses qw( -utf-8) but another does not: +use CGI; +use CGI qw( -utf8 ); (blocker) 3) Unit tests ... all passing! 4) Feature tests OPAC a) I feel we don't need the "Pages" heading. It only takes up space and states the obvious. (non-blocker... but navigation needs work, see below) b) OpacMainUserBlock should not show on the pages in my opinion. Libraries use it to place text on the start page instead of using the news feature, which means it repeats on all pages. I'd really like to see this changed. c) Personally I'd change "Location" to "Interface" to avoid confusion with the item value in translations. But I see it's the same on news. So out of scope here. d) Parent pages can be deleted without warning. Childs will be moved to top level. This is acceptable behaviour as they are not left orphaned. But it might be something to improve on in the future. e) In the test plan and in the code I see there should be a confirmation message on deleting a single or multiple entries, but this doesn't work. (blocker) f) Navigation looks strange when working with multiple levels. Repeating the level before seems unnecessary and only takes up space? Level 1 Level 1 › Level 2 Level 2 › Level 3 Also, clicking on the different links, sometimes a part of the navigation disappears. I think this is a little too wonky to pass. Can you take another look please? (blocker) Intranet Where do the pages appear in the intranet? Generally I think this will be a great and really helpful addition and looking forward to return to this! Created attachment 78232 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 78233 [details] [review] Bug 15326: (follow-up) Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 78234 [details] [review] Bug 15326: (follow-up) utf8 to utf8mb4 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 78235 [details] [review] Bug 15326: (follow-up) Move js to footer, use Asset plugin Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 78236 [details] [review] Bug 15326: (follow-up) Don't make links to unpublished pages in opac Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 78237 [details] [review] Bug 15326: (follow-up) Fix tools menu highlighting Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Rebased, working on things in Comment 36 Created attachment 78238 [details] [review] Bug 15326: (follow-up) Code review fixes Addresses the following issues: b) In opac-cmspages there is: + flagsrequired => { borrow => 1 }, The borrow permission has been removed since. (blocker) c) Comment was removed unnecessarily, just needs to be moved into its right spot below again: -# If GoogleIndicTransliteration system preference is On Set parameter to load Google's javascript in OPAC search screens d) One file uses qw( -utf-8) but another does not: +use CGI; +use CGI qw( -utf8 ); (blocker) Created attachment 78240 [details] [review] Bug 15326: (follow-up) Interface fixes This patch addresses the following: a) I feel we don't need the "Pages" heading. It only takes up space and states the obvious. (non-blocker... but navigation needs work, see below) b) OpacMainUserBlock should not show on the pages in my opinion. Libraries use it to place text on the start page instead of using the news feature, which means it repeats on all pages. I'd really like to see this changed. c) Personally I'd change "Location" to "Interface" to avoid confusion with the item value in translations. But I see it's the same on news. So out of scope here. d) Parent pages can be deleted without warning. Childs will be moved to top level. This is acceptable behaviour as they are not left orphaned. But it might be something to improve on in the future. e) In the test plan and in the code I see there should be a confirmation message on deleting a single or multiple entries, but this doesn't work. (blocker) Created attachment 79269 [details] [review] Bug 15326: (follow-up) Pages navigation fixes This is the best I've managed to do with the navigation. Would be happy for someone else to give it a go because I'm aware that some parent links repeat. Created attachment 83318 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 83319 [details] [review] Bug 15326: (follow-up) Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 83320 [details] [review] Bug 15326: (follow-up) utf8 to utf8mb4 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 83321 [details] [review] Bug 15326: (follow-up) Move js to footer, use Asset plugin Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 83322 [details] [review] Bug 15326: (follow-up) Don't make links to unpublished pages in opac Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 83323 [details] [review] Bug 15326: (follow-up) Fix tools menu highlighting Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 83324 [details] [review] Bug 15326: (follow-up) Code review fixes Addresses the following issues: b) In opac-cmspages there is: + flagsrequired => { borrow => 1 }, The borrow permission has been removed since. (blocker) c) Comment was removed unnecessarily, just needs to be moved into its right spot below again: -# If GoogleIndicTransliteration system preference is On Set parameter to load Google's javascript in OPAC search screens d) One file uses qw( -utf-8) but another does not: +use CGI; +use CGI qw( -utf8 ); (blocker) Created attachment 83325 [details] [review] Bug 15326: (follow-up) Interface fixes This patch addresses the following: a) I feel we don't need the "Pages" heading. It only takes up space and states the obvious. (non-blocker... but navigation needs work, see below) b) OpacMainUserBlock should not show on the pages in my opinion. Libraries use it to place text on the start page instead of using the news feature, which means it repeats on all pages. I'd really like to see this changed. c) Personally I'd change "Location" to "Interface" to avoid confusion with the item value in translations. But I see it's the same on news. So out of scope here. d) Parent pages can be deleted without warning. Childs will be moved to top level. This is acceptable behaviour as they are not left orphaned. But it might be something to improve on in the future. e) In the test plan and in the code I see there should be a confirmation message on deleting a single or multiple entries, but this doesn't work. (blocker) Created attachment 83326 [details] [review] Bug 15326: (follow-up) Pages navigation fixes This is the best I've managed to do with the navigation. Would be happy for someone else to give it a go because I'm aware that some parent links repeat. This has been rebased and can be tested. Navigation could still use work. Created attachment 83404 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83405 [details] [review] Bug 15326: (follow-up) Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83406 [details] [review] Bug 15326: (follow-up) utf8 to utf8mb4 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83407 [details] [review] Bug 15326: (follow-up) Move js to footer, use Asset plugin Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83408 [details] [review] Bug 15326: (follow-up) Don't make links to unpublished pages in opac Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83409 [details] [review] Bug 15326: (follow-up) Fix tools menu highlighting Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83410 [details] [review] Bug 15326: (follow-up) Code review fixes Addresses the following issues: b) In opac-cmspages there is: + flagsrequired => { borrow => 1 }, The borrow permission has been removed since. (blocker) c) Comment was removed unnecessarily, just needs to be moved into its right spot below again: -# If GoogleIndicTransliteration system preference is On Set parameter to load Google's javascript in OPAC search screens d) One file uses qw( -utf-8) but another does not: +use CGI; +use CGI qw( -utf8 ); (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83411 [details] [review] Bug 15326: (follow-up) Interface fixes This patch addresses the following: a) I feel we don't need the "Pages" heading. It only takes up space and states the obvious. (non-blocker... but navigation needs work, see below) b) OpacMainUserBlock should not show on the pages in my opinion. Libraries use it to place text on the start page instead of using the news feature, which means it repeats on all pages. I'd really like to see this changed. c) Personally I'd change "Location" to "Interface" to avoid confusion with the item value in translations. But I see it's the same on news. So out of scope here. d) Parent pages can be deleted without warning. Childs will be moved to top level. This is acceptable behaviour as they are not left orphaned. But it might be something to improve on in the future. e) In the test plan and in the code I see there should be a confirmation message on deleting a single or multiple entries, but this doesn't work. (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83412 [details] [review] Bug 15326: (follow-up) Pages navigation fixes This is the best I've managed to do with the navigation. Would be happy for someone else to give it a go because I'm aware that some parent links repeat. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Conflicts in the Schema changes part (should be a separate patch) Can you please fix and maybe separate these changes out? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 15326: Add CMS feature Applying: Bug 15326: (follow-up) Add CMS feature Using index info to reconstruct a base tree... M Koha/Schema/Result/Branch.pm Falling back to patching base and 3-way merge... Removing t/Pages.t Auto-merging Koha/Schema/Result/Branch.pm CONFLICT (content): Merge conflict in Koha/Schema/Result/Branch.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 15326: (follow-up) Add CMS feature The copy of the patch that failed is found in: .git/rebase-apply/patch 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". Patch left in /tmp/Bug-15326-follow-up-Add-CMS-feature-E4XbRe.patch Created attachment 84682 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 84683 [details] [review] Bug 15326: (follow-up) Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 84684 [details] [review] Bug 15326: (follow-up) utf8 to utf8mb4 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 84685 [details] [review] Bug 15326: (follow-up) Move js to footer, use Asset plugin Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 84686 [details] [review] Bug 15326: (follow-up) Don't make links to unpublished pages in opac Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 84687 [details] [review] Bug 15326: (follow-up) Fix tools menu highlighting Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 84688 [details] [review] Bug 15326: (follow-up) Code review fixes Addresses the following issues: b) In opac-cmspages there is: + flagsrequired => { borrow => 1 }, The borrow permission has been removed since. (blocker) c) Comment was removed unnecessarily, just needs to be moved into its right spot below again: -# If GoogleIndicTransliteration system preference is On Set parameter to load Google's javascript in OPAC search screens d) One file uses qw( -utf-8) but another does not: +use CGI; +use CGI qw( -utf8 ); (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 84689 [details] [review] Bug 15326: (follow-up) Interface fixes This patch addresses the following: a) I feel we don't need the "Pages" heading. It only takes up space and states the obvious. (non-blocker... but navigation needs work, see below) b) OpacMainUserBlock should not show on the pages in my opinion. Libraries use it to place text on the start page instead of using the news feature, which means it repeats on all pages. I'd really like to see this changed. c) Personally I'd change "Location" to "Interface" to avoid confusion with the item value in translations. But I see it's the same on news. So out of scope here. d) Parent pages can be deleted without warning. Childs will be moved to top level. This is acceptable behaviour as they are not left orphaned. But it might be something to improve on in the future. e) In the test plan and in the code I see there should be a confirmation message on deleting a single or multiple entries, but this doesn't work. (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 84690 [details] [review] Bug 15326: (follow-up) Pages navigation fixes This is the best I've managed to do with the navigation. Would be happy for someone else to give it a go because I'm aware that some parent links repeat. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 84691 [details] [review] Bug 15326: Schema changes (In reply to Katrin Fischer from comment #66) > Conflicts in the Schema changes part (should be a separate patch) Can you > please fix and maybe separate these changes out? > > Apply? [(y)es, (n)o, (i)nteractive] y > Applying: Bug 15326: Add CMS feature > Applying: Bug 15326: (follow-up) Add CMS feature > Using index info to reconstruct a base tree... > M Koha/Schema/Result/Branch.pm > Falling back to patching base and 3-way merge... > Removing t/Pages.t > Auto-merging Koha/Schema/Result/Branch.pm > CONFLICT (content): Merge conflict in Koha/Schema/Result/Branch.pm > error: Failed to merge in the changes. > Patch failed at 0001 Bug 15326: (follow-up) Add CMS feature > The copy of the patch that failed is found in: .git/rebase-apply/patch > 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". > Patch left in /tmp/Bug-15326-follow-up-Add-CMS-feature-E4XbRe.patch Rebased and moved schema fixes into a separate patch Hi Aleisha, you can usually set back to "Signed off" after a rebase if it was before (unless you feel the changes were too heavy and should be retested) (In reply to Katrin Fischer from comment #78) > Hi Aleisha, you can usually set back to "Signed off" after a rebase if it > was before (unless you feel the changes were too heavy and should be > retested) Set back to Signed off! *** Bug 12806 has been marked as a duplicate of this bug. *** Applying: Bug 15326: Add CMS feature .git/rebase-apply/patch:191: tab in indent. -asc => [qw/parent location branchcode lang sortorder/] .git/rebase-apply/patch:241: tab in indent. my ( $self, $id ) = @_; .git/rebase-apply/patch:243: tab in indent. carp( 'Missing CMS page id parameter in Koha::Pages::page' ) unless defined $id; .git/rebase-apply/patch:245: tab in indent. # Probably want to merge these to a single query: .git/rebase-apply/patch:246: tab in indent. my $top_links = $self->_resultset()->search( warning: squelched 77 whitespace errors warning: 82 lines add whitespace errors. Using index info to reconstruct a base tree... M installer/data/mysql/kohastructure.sql M installer/data/mysql/userpermissions.sql M koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc M koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt M opac/opac-main.pl .git/rebase-apply/patch:191: tab in indent. -asc => [qw/parent location branchcode lang sortorder/] .git/rebase-apply/patch:241: tab in indent. my ( $self, $id ) = @_; .git/rebase-apply/patch:243: tab in indent. carp( 'Missing CMS page id parameter in Koha::Pages::page' ) unless defined $id; .git/rebase-apply/patch:245: tab in indent. # Probably want to merge these to a single query: .git/rebase-apply/patch:246: tab in indent. my $top_links = $self->_resultset()->search( warning: squelched 77 whitespace errors warning: 82 lines applied after fixing whitespace errors. CONFLICT (content): Merge conflict in opac/opac-main.pl CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc Patch failed at 0001 Bug 15326: Add CMS feature Created attachment 87494 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 87495 [details] [review] Bug 15326: (follow-up) Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 87496 [details] [review] Bug 15326: (follow-up) utf8 to utf8mb4 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 87497 [details] [review] Bug 15326: (follow-up) Move js to footer, use Asset plugin Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 87498 [details] [review] Bug 15326: (follow-up) Don't make links to unpublished pages in opac Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 87499 [details] [review] Bug 15326: (follow-up) Fix tools menu highlighting Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 87500 [details] [review] Bug 15326: (follow-up) Code review fixes Addresses the following issues: b) In opac-cmspages there is: + flagsrequired => { borrow => 1 }, The borrow permission has been removed since. (blocker) c) Comment was removed unnecessarily, just needs to be moved into its right spot below again: -# If GoogleIndicTransliteration system preference is On Set parameter to load Google's javascript in OPAC search screens d) One file uses qw( -utf-8) but another does not: +use CGI; +use CGI qw( -utf8 ); (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 87501 [details] [review] Bug 15326: (follow-up) Interface fixes This patch addresses the following: a) I feel we don't need the "Pages" heading. It only takes up space and states the obvious. (non-blocker... but navigation needs work, see below) b) OpacMainUserBlock should not show on the pages in my opinion. Libraries use it to place text on the start page instead of using the news feature, which means it repeats on all pages. I'd really like to see this changed. c) Personally I'd change "Location" to "Interface" to avoid confusion with the item value in translations. But I see it's the same on news. So out of scope here. d) Parent pages can be deleted without warning. Childs will be moved to top level. This is acceptable behaviour as they are not left orphaned. But it might be something to improve on in the future. e) In the test plan and in the code I see there should be a confirmation message on deleting a single or multiple entries, but this doesn't work. (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 87502 [details] [review] Bug 15326: (follow-up) Pages navigation fixes This is the best I've managed to do with the navigation. Would be happy for someone else to give it a go because I'm aware that some parent links repeat. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 87503 [details] [review] Bug 15326: Schema changes Rebased to get this moving again. A quick look shows a few issues which need to be addressed: YUI grids in the staff client, LESS in the OPAC, previously deleted JS in tools-menu.inc. I'm going to submit a follow-up, but marking failed until then. Created attachment 87608 [details] [review] Bug 15326: (follow-up) Redindent new CMS page templates This patch is an opinionated reindent of new and changed templates related to the CMS feature. I think it would be nice for these templates to start out with good indentation. To test, apply the patch and test the CMS feature. There should be no visible change to the way it looks or the way it works. Created attachment 87609 [details] [review] Bug 15326: (follow-up) Corrections to staff client templates This patch makes some corrections to the staff client's CMS-related templates: - Add missing Template Toolkit filters. - Remove JavaScript from tools-menu.inc. This must have crept in during a rebase. - Add navigation link highlighting to tools-menu.js instead. - Convert tools/cmspages.tt to Bootstrap grid. - Correct toolbar button class. - Move some column sorting configuration out of JS and into table markup. - Move "Select all" / "Clear all" links to the top of the table to match similar interfaces. - Change the Action menu's "Intranet" and "OPAC" links to be "View" and "View in OPAC." The OPAC view now opens a new window. To test, apply the patches and test the Pages feature in the staff client: Adding, editing, deleting, viewing. For each view, test that the page layout adjusts to various browser widths. Created attachment 87610 [details] [review] Bug 15326: (follow-up) Remove obsolete LESS file, add TT filters Previous versions of this patch set adds a LESS file to the OPAC, which is now obsolete. It should be removed. In addition, the change made to opac.less contained CSS which didn't apply to any existing markup, so it need not be added to opac.scss. The patch also adds missing Template Toolkit filters to template files. None of these changes should be visible in testing. OPAC CMS pages should function as usual. I fixed template issues reported by the QA tool, but not Perl errors. I'm leaving this in 'failed QA' because of that. One thing that I would really like to see improved: I think the navigation menu in the OPAC doesn't work the way it should. For example, this menu based on pages I was testing with: - About us - About us › Nelsonville Public Library - About us › Athens Public Library - About us › The Plains Public Library Why is "About us" repeated for each sub-page? Why aren't sub-pages displayed as a list nested inside the list containing the parent link? Based on other CMS navigation menus I would have expected this: <ul> <li>About us <ul> <li>Nelsonville Public Library</li> <li>Athens Public Library</li> <li>The Plains Public Library</li> </ul> </li> </ul> This is semantically more correct and provides more flexibility for CSS styling because you can leverage element nesting. The other thing I see which seems wrong is that if I'm looking at the parent page ("About us" in the example above) I don't see the sub-pages. I would expect them to either be shown all the time or only when viewing the parent page or its sub-pages. Created attachment 87617 [details] [review] Bug 15326: (follow-up) Remove obsolete LESS file, add TT filters Previous versions of this patch set adds a LESS file to the OPAC, which is now obsolete. It should be removed. In addition, the change made to opac.less contained CSS which didn't apply to any existing markup, so it need not be added to opac.scss. The patch also adds missing Template Toolkit filters to template files. None of these changes should be visible in testing. OPAC CMS pages should function as usual. Created attachment 96084 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96085 [details] [review] Bug 15326: (follow-up) Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96086 [details] [review] Bug 15326: (follow-up) utf8 to utf8mb4 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96087 [details] [review] Bug 15326: (follow-up) Move js to footer, use Asset plugin Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96088 [details] [review] Bug 15326: (follow-up) Don't make links to unpublished pages in opac Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96089 [details] [review] Bug 15326: (follow-up) Fix tools menu highlighting Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96090 [details] [review] Bug 15326: (follow-up) Code review fixes Addresses the following issues: b) In opac-cmspages there is: + flagsrequired => { borrow => 1 }, The borrow permission has been removed since. (blocker) c) Comment was removed unnecessarily, just needs to be moved into its right spot below again: -# If GoogleIndicTransliteration system preference is On Set parameter to load Google's javascript in OPAC search screens d) One file uses qw( -utf-8) but another does not: +use CGI; +use CGI qw( -utf8 ); (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96091 [details] [review] Bug 15326: (follow-up) Interface fixes This patch addresses the following: a) I feel we don't need the "Pages" heading. It only takes up space and states the obvious. (non-blocker... but navigation needs work, see below) b) OpacMainUserBlock should not show on the pages in my opinion. Libraries use it to place text on the start page instead of using the news feature, which means it repeats on all pages. I'd really like to see this changed. c) Personally I'd change "Location" to "Interface" to avoid confusion with the item value in translations. But I see it's the same on news. So out of scope here. d) Parent pages can be deleted without warning. Childs will be moved to top level. This is acceptable behaviour as they are not left orphaned. But it might be something to improve on in the future. e) In the test plan and in the code I see there should be a confirmation message on deleting a single or multiple entries, but this doesn't work. (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96092 [details] [review] Bug 15326: (follow-up) Pages navigation fixes This is the best I've managed to do with the navigation. Would be happy for someone else to give it a go because I'm aware that some parent links repeat. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96093 [details] [review] Bug 15326: (follow-up) Redindent new CMS page templates This patch is an opinionated reindent of new and changed templates related to the CMS feature. I think it would be nice for these templates to start out with good indentation. To test, apply the patch and test the CMS feature. There should be no visible change to the way it looks or the way it works. Created attachment 96094 [details] [review] Bug 15326: (follow-up) Corrections to staff client templates This patch makes some corrections to the staff client's CMS-related templates: - Add missing Template Toolkit filters. - Remove JavaScript from tools-menu.inc. This must have crept in during a rebase. - Add navigation link highlighting to tools-menu.js instead. - Convert tools/cmspages.tt to Bootstrap grid. - Correct toolbar button class. - Move some column sorting configuration out of JS and into table markup. - Move "Select all" / "Clear all" links to the top of the table to match similar interfaces. - Change the Action menu's "Intranet" and "OPAC" links to be "View" and "View in OPAC." The OPAC view now opens a new window. To test, apply the patches and test the Pages feature in the staff client: Adding, editing, deleting, viewing. For each view, test that the page layout adjusts to various browser widths. Created attachment 96095 [details] [review] Bug 15326: (follow-up) Remove obsolete LESS file, add TT filters Previous versions of this patch set adds a LESS file to the OPAC, which is now obsolete. It should be removed. In addition, the change made to opac.less contained CSS which didn't apply to any existing markup, so it need not be added to opac.scss. The patch also adds missing Template Toolkit filters to template files. None of these changes should be visible in testing. OPAC CMS pages should function as usual. Created attachment 96096 [details] [review] Bug 15326: Schema changes Created attachment 96097 [details] [review] Bug 15326: Fix QA errors Created attachment 96098 [details] [review] Bug 15326: Fix filters and adjust tiny_mce assets I did what I could to move this along, rebased, cleared QA errors, fixed filters Issues I had: 1 - The content field is hidden when I try to make a new page 2 - I cannot add a page under strict mode - the call to update_or_create doesn't like id as '' - if i try ot make it undef when not set, then the dbix gets upset about using key primary with no values It appears that title_link is enforced as unique in the module, though not in the table. If we do enforce this we can add a new constraint on that and use if for update/create but it requires more work https://metacpan.org/pod/DBIx::Class::ResultSet#update_or_create Note: Take care when using update_or_create with a table having columns with default values that you intend to be automatically supplied by the database (e.g. an auto_increment primary key column). In normal usage, the value of such columns should NOT be included at all in the call to update_or_create, even when set to undef. Shouldn't hitting an unpublished page return a 404 error? Created attachment 97237 [details] [review] Bug 15326: (follow-up) Template tweaks This patch makes some tweaks to the staff-side template: - Fix some indentation - Add some markup comments - Fix some HTML validation errors: Eliminate duplicate id attributes - Expand the descriptiveness of <title>, breadcrumbs, and form legend - Increase size of link and title fields; Add maxlength attribute to title field. - Move title and link fields to the second column of the table -- more consistent with other tables where the main link is in the first column. - Link parent page information to corresponding edit page - Correct incorrectly filtered action links - Correct default table sort - Fix TinyMCE configuration - I copied some configuration code from another page which was updated more recently. To test, apply the patch and test the process of adding, editing, and deleting pages. Test all the options in the "Actions" menu. These patches still apply, so it's a great time for more testing and fixes! (In reply to Nick Clemens from comment #115) > 1 - The content field is hidden when I try to make a new page I think my patch fixes this issue. Created attachment 101565 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101566 [details] [review] Bug 15326: (follow-up) Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101567 [details] [review] Bug 15326: (follow-up) utf8 to utf8mb4 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101568 [details] [review] Bug 15326: (follow-up) Move js to footer, use Asset plugin Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101569 [details] [review] Bug 15326: (follow-up) Don't make links to unpublished pages in opac Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101570 [details] [review] Bug 15326: (follow-up) Fix tools menu highlighting Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101571 [details] [review] Bug 15326: (follow-up) Code review fixes Addresses the following issues: b) In opac-cmspages there is: + flagsrequired => { borrow => 1 }, The borrow permission has been removed since. (blocker) c) Comment was removed unnecessarily, just needs to be moved into its right spot below again: -# If GoogleIndicTransliteration system preference is On Set parameter to load Google's javascript in OPAC search screens d) One file uses qw( -utf-8) but another does not: +use CGI; +use CGI qw( -utf8 ); (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101572 [details] [review] Bug 15326: (follow-up) Interface fixes This patch addresses the following: a) I feel we don't need the "Pages" heading. It only takes up space and states the obvious. (non-blocker... but navigation needs work, see below) b) OpacMainUserBlock should not show on the pages in my opinion. Libraries use it to place text on the start page instead of using the news feature, which means it repeats on all pages. I'd really like to see this changed. c) Personally I'd change "Location" to "Interface" to avoid confusion with the item value in translations. But I see it's the same on news. So out of scope here. d) Parent pages can be deleted without warning. Childs will be moved to top level. This is acceptable behaviour as they are not left orphaned. But it might be something to improve on in the future. e) In the test plan and in the code I see there should be a confirmation message on deleting a single or multiple entries, but this doesn't work. (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101573 [details] [review] Bug 15326: (follow-up) Pages navigation fixes This is the best I've managed to do with the navigation. Would be happy for someone else to give it a go because I'm aware that some parent links repeat. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 101574 [details] [review] Bug 15326: (follow-up) Redindent new CMS page templates This patch is an opinionated reindent of new and changed templates related to the CMS feature. I think it would be nice for these templates to start out with good indentation. To test, apply the patch and test the CMS feature. There should be no visible change to the way it looks or the way it works. Created attachment 101575 [details] [review] Bug 15326: (follow-up) Corrections to staff client templates This patch makes some corrections to the staff client's CMS-related templates: - Add missing Template Toolkit filters. - Remove JavaScript from tools-menu.inc. This must have crept in during a rebase. - Add navigation link highlighting to tools-menu.js instead. - Convert tools/cmspages.tt to Bootstrap grid. - Correct toolbar button class. - Move some column sorting configuration out of JS and into table markup. - Move "Select all" / "Clear all" links to the top of the table to match similar interfaces. - Change the Action menu's "Intranet" and "OPAC" links to be "View" and "View in OPAC." The OPAC view now opens a new window. To test, apply the patches and test the Pages feature in the staff client: Adding, editing, deleting, viewing. For each view, test that the page layout adjusts to various browser widths. Created attachment 101576 [details] [review] Bug 15326: (follow-up) Remove obsolete LESS file, add TT filters Previous versions of this patch set adds a LESS file to the OPAC, which is now obsolete. It should be removed. In addition, the change made to opac.less contained CSS which didn't apply to any existing markup, so it need not be added to opac.scss. The patch also adds missing Template Toolkit filters to template files. None of these changes should be visible in testing. OPAC CMS pages should function as usual. Created attachment 101577 [details] [review] Bug 15326: Fix QA errors Created attachment 101578 [details] [review] Bug 15326: Fix filters and adjust tiny_mce assets Created attachment 101579 [details] [review] Bug 15326: (follow-up) Template tweaks This patch makes some tweaks to the staff-side template: - Fix some indentation - Add some markup comments - Fix some HTML validation errors: Eliminate duplicate id attributes - Expand the descriptiveness of <title>, breadcrumbs, and form legend - Increase size of link and title fields; Add maxlength attribute to title field. - Move title and link fields to the second column of the table -- more consistent with other tables where the main link is in the first column. - Link parent page information to corresponding edit page - Correct incorrectly filtered action links - Correct default table sort - Fix TinyMCE configuration - I copied some configuration code from another page which was updated more recently. To test, apply the patch and test the process of adding, editing, and deleting pages. Test all the options in the "Actions" menu. Created attachment 101580 [details] [review] Bug 15326: DO NOT PUSH - Schema changes Rebased, found issues: Parent child links are a confusing: List display parent name needlessly Cannot see children from parent in breadcrumbs Can access unpublished pages Library specific pages are listed for all branches Staff side link should use link title not page title Can view staff pages on opac Created attachment 128341 [details] [review] Bug 15326: Add CMS feature INCOMPLETE - For evaluation purposes only! Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 128342 [details] [review] Bug 15326: (follow-up) Add CMS feature This patch builds on Martin Persson's patch. I made a few changes that improve the patch and make this feature functional. The only thing I am not completely satisfied with (and doesn't reliably work) is the navigation for the CMS pages on both the staff side and the OPAC side. This needs work. Otherwise the CMS feature works. - Make sure you update database after applying patch - After adding a few pages, confirm that filtering results works (play with different combinations of display location, library and language) - Confirm that pages with specific locations ONLY show up in those locations, i.e. OPAC pages can only be accessed in OPAC and staff interface pages can only be accessed on staff client - Confirm that table is showing correct data, particularly language, location and library - Confirm that if page has not been published and user does not have permission to edit pages, page content will not display on OPAC side. On staff side, a message will show saying that page has not been published - Confirm pagination for table shows and works - Confirm deleting pages works as expected (confirm message before delete) - Confirm link title must be unique when adding new pages - Confirm that editing the details of an existing page works - Confirm that adding a child page will only work if the location of child page matches location of parent page, i.e., if parent page is an OPAC page then child page must also be OPAC page. if parent page is in all locations then child page can be in all or either interface(s) - Confirm Select All/Clear All toggle works - Confirm viewing of CMS pages in OPAC and in staff client works and looks nice Sponsored-by: Region Halland Bug 15326: [FOLLOW-UP] Many fixes This patch: - moves action buttons into a dropdown, adds a delete button to dropdown - fixes the tests (they should now all pass) - fixes qa tool problems - removes the Koha::CmsPages->remove() method (unnecessary) - makes changes to the Koha::CmsPages->add() method (now simpler, easier to test) This is now ready to test. Sponsored-by: Catalyst IT Bug 15326: [FOLLOW-UP] More fixes This patch removes the t/Pages.t file because it isn't required, and other unused methods. This patch is ready to test. The appropriate tests are in t/db_dependent/Pages.t Bug 15326: [FOLLOW-UP] adding and editing fixes Sorry, did not see the second half of your comment. This patch should fix the adding and editing issues in Comment 16. Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing Fixes issues from Comment 19, please test Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 128343 [details] [review] Bug 15326: (follow-up) utf8 to utf8mb4 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 128344 [details] [review] Bug 15326: (follow-up) Move js to footer, use Asset plugin Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 128345 [details] [review] Bug 15326: (follow-up) Don't make links to unpublished pages in opac Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 128346 [details] [review] Bug 15326: (follow-up) Fix tools menu highlighting Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 128347 [details] [review] Bug 15326: (follow-up) Code review fixes Addresses the following issues: b) In opac-cmspages there is: + flagsrequired => { borrow => 1 }, The borrow permission has been removed since. (blocker) c) Comment was removed unnecessarily, just needs to be moved into its right spot below again: -# If GoogleIndicTransliteration system preference is On Set parameter to load Google's javascript in OPAC search screens d) One file uses qw( -utf-8) but another does not: +use CGI; +use CGI qw( -utf8 ); (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 128348 [details] [review] Bug 15326: (follow-up) Interface fixes This patch addresses the following: a) I feel we don't need the "Pages" heading. It only takes up space and states the obvious. (non-blocker... but navigation needs work, see below) b) OpacMainUserBlock should not show on the pages in my opinion. Libraries use it to place text on the start page instead of using the news feature, which means it repeats on all pages. I'd really like to see this changed. c) Personally I'd change "Location" to "Interface" to avoid confusion with the item value in translations. But I see it's the same on news. So out of scope here. d) Parent pages can be deleted without warning. Childs will be moved to top level. This is acceptable behaviour as they are not left orphaned. But it might be something to improve on in the future. e) In the test plan and in the code I see there should be a confirmation message on deleting a single or multiple entries, but this doesn't work. (blocker) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 128349 [details] [review] Bug 15326: (follow-up) Pages navigation fixes This is the best I've managed to do with the navigation. Would be happy for someone else to give it a go because I'm aware that some parent links repeat. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 128350 [details] [review] Bug 15326: (follow-up) Redindent new CMS page templates This patch is an opinionated reindent of new and changed templates related to the CMS feature. I think it would be nice for these templates to start out with good indentation. To test, apply the patch and test the CMS feature. There should be no visible change to the way it looks or the way it works. Created attachment 128351 [details] [review] Bug 15326: (follow-up) Corrections to staff client templates This patch makes some corrections to the staff client's CMS-related templates: - Add missing Template Toolkit filters. - Remove JavaScript from tools-menu.inc. This must have crept in during a rebase. - Add navigation link highlighting to tools-menu.js instead. - Convert tools/cmspages.tt to Bootstrap grid. - Correct toolbar button class. - Move some column sorting configuration out of JS and into table markup. - Move "Select all" / "Clear all" links to the top of the table to match similar interfaces. - Change the Action menu's "Intranet" and "OPAC" links to be "View" and "View in OPAC." The OPAC view now opens a new window. To test, apply the patches and test the Pages feature in the staff client: Adding, editing, deleting, viewing. For each view, test that the page layout adjusts to various browser widths. Created attachment 128352 [details] [review] Bug 15326: (follow-up) Remove obsolete LESS file, add TT filters Previous versions of this patch set adds a LESS file to the OPAC, which is now obsolete. It should be removed. In addition, the change made to opac.less contained CSS which didn't apply to any existing markup, so it need not be added to opac.scss. The patch also adds missing Template Toolkit filters to template files. None of these changes should be visible in testing. OPAC CMS pages should function as usual. Created attachment 128353 [details] [review] Bug 15326: Fix QA errors Created attachment 128354 [details] [review] Bug 15326: Fix filters and adjust tiny_mce assets Created attachment 128355 [details] [review] Bug 15326: (follow-up) Template tweaks This patch makes some tweaks to the staff-side template: - Fix some indentation - Add some markup comments - Fix some HTML validation errors: Eliminate duplicate id attributes - Expand the descriptiveness of <title>, breadcrumbs, and form legend - Increase size of link and title fields; Add maxlength attribute to title field. - Move title and link fields to the second column of the table -- more consistent with other tables where the main link is in the first column. - Link parent page information to corresponding edit page - Correct incorrectly filtered action links - Correct default table sort - Fix TinyMCE configuration - I copied some configuration code from another page which was updated more recently. To test, apply the patch and test the process of adding, editing, and deleting pages. Test all the options in the "Actions" menu. Created attachment 128356 [details] [review] Bug 15326: (follow-up) WIP Fixing breadcrumbs Hiding private/unpublished pages etc Still working on pages navigation Created attachment 128357 [details] [review] Bug 15326: Schema updates Aleisha, with the recent "additional contents" changes we could have a new type of content "page". They would be stored in the same DB table and would profit from the other possibilities (translation, etc.) Is it something that could work for you? (In reply to Jonathan Druart from comment #153) > Aleisha, with the recent "additional contents" changes we could have a new > type of content "page". They would be stored in the same DB table and would > profit from the other possibilities (translation, etc.) > > Is it something that could work for you? Yes, I was thinking the same. Actually I am already using the news database as a simple cms for some local pages. Perhaps some of the additional features(?) here could be incorporated. Created attachment 128585 [details] [review] Bug 15326: WIP: Using AdditionalContents for custom pages on OPAC Created attachment 128642 [details] [review] Bug 15326: Using AdditionalContents for custom pages on OPAC This page utilises the Additional Contents feature to add custom pages to the staff client and the OPAC in the user's desired language. To test: 1. Apply patch and restart services 2. In the staff client, go to Tools, then go to the new 'Pages' page 3. Add a new page to display on both the staff client and OPAC. 4. Notice the 'View' button in the Actions column. This button should only show when viewing Pages, it should not show when viewing HTML customisations or News. Test that the main View button links you to the page in the staff client. Test that clicking the arrow shows a button to view the page in the OPAC, and clicking it opens the OPAC in a new tab. 5. When viewing your new page in the staff client, confirm a Pages navigation shows in the left navigation. 6. When viewing your new page in the OPAC, confirm a Pages navigation shows in the left navigation. Confirm pages are also linked in the masthead links (i.e. with Advanced search and Libraries etc). 7. Confirm that any pages made for the staff client only do not show in the OPAC, and that any pages made for the OPAC only do not show in the staff client. 8. Pages that can be viewed on the staff client will also show under the Pages link on the Tools main page. Sponsored-by: Catalyst IT Aleisha, This is a very nice looking feature! A couple things I noticed while testing: 1. If I make a page for the OPAC or both and click the 'View' link I get a 404 error at: http://localhost:8080/cgi-bin/koha/page.pl?page_id=7 In the OPACMoreSearches line the link is build correctly as http://localhost:8080/cgi-bin/koha/opac-page.pl?page_id=7 and I can see it without 404. 2. IF I click on the dropdown-toggle arrow next to the view button I see the 2 options 'View on OPAC' and 'View on staff client'. They work good. However, if the page is set for staff only or OPAC only when I click the button nothing happens. Thanks for working on this. Created attachment 136432 [details] [review] Bug 15326: Using AdditionalContents for custom pages on OPAC This page utilises the Additional Contents feature to add custom pages to the staff client and the OPAC in the user's desired language. To test: 1. Apply patch and restart services 2. In the staff client, go to Tools, then go to the new 'Pages' page 3. Add a new page to display on both the staff client and OPAC. 4. Notice the 'View' button in the Actions column. This button should only show when viewing Pages, it should not show when viewing HTML customisations or News. Test that the main View button links you to the page in the staff client. Test that clicking the arrow shows a button to view the page in the OPAC, and clicking it opens the OPAC in a new tab. 5. When viewing your new page in the staff client, confirm a Pages navigation shows in the left navigation. 6. When viewing your new page in the OPAC, confirm a Pages navigation shows in the left navigation. Confirm pages are also linked in the masthead links (i.e. with Advanced search and Libraries etc). 7. Confirm that any pages made for the staff client only do not show in the OPAC, and that any pages made for the OPAC only do not show in the staff client. 8. Pages that can be viewed on the staff client will also show under the Pages link on the Tools main page. Sponsored-by: Chartered Accountants Australia and New Zealand 1. Actions menu for pages: "View on staff client" should be "View on staff interface" (see terminology list). 2. If I have a default version and an English version, when the English version has a different title: - View - without selecting from the drop down list, View on OPAC, and View on staff client: get a "This page does not exist." error, but the title is listed under the pages heading - the URLs are different (for example in the OPAC the page opens to this URL http://127.0.0.1:8080/cgi-bin/koha/opac-page.pl?page_id=3 and for the link under Pages it is http://127.0.0.1:8080/cgi-bin/koha/opac-page.pl?page_id=4) - If the titles are the same, it appears as expected. 3. The bulleted list of pages is not indetned/aligned under "Write custom pages" Just some thoughts:* 1. Is it necessary to list the pages under Tools > Additional tools > Pages? (Unless this is the only way to view them.) Couldn't these just be accessed from the filter like you see when you go into the pages page? 2. When viewing the pages from the bulleted list under Pages, I found the listing of pages in the left side bar as well a bit confusing. Is this the only way to see the pages targeted for the staff interface? 3. Preview - it just shows the default, should the other languages be shown? 4. If you have a lot of pages, being listed in the OPAC in the masthead could get rather long. * Just some additional things I noticed - I'm not tied to them in any way, just some first impressions! I didn't install and test with another language installed. Created attachment 136757 [details] [review] Bug 15326: Using AdditionalContents for custom pages on OPAC This page utilises the Additional Contents feature to add custom pages to the staff client and the OPAC in the user's desired language. To test: 1. Apply patch and restart services 2. In the staff client, go to Tools, then go to the new 'Pages' page 3. Add a new page to display on both the staff client and OPAC. 4. Confirm the URLs in the Page URL column work as expected. 5. Confirm that any pages made for the staff client only do not show in the OPAC, and that any pages made for the OPAC only do not show in the staff client. 6. Confirm that pages do not show on the OPAC main page where news items show. Confirm news items show on the OPAC main page as expected. Sponsored-by: Chartered Accountants Australia and New Zealand Thanks for testing David. I changed 'View on staff client' to 'View on librarian interface' to match the language used elsewhere in the Additional Contents tools. I decided to remove the navigation for pages as, you're right, they didn't really make sense. I also removed the View buttons. Instead I added a new column to the Pages table to include the new Page URLs so they can be inserted where the library chooses, and be used to View. Created attachment 136768 [details] [review] Bug 15326: Using AdditionalContents for custom pages on OPAC This page utilises the Additional Contents feature to add custom pages to the staff client and the OPAC in the user's desired language. To test: 1. Apply patch and restart services 2. In the staff client, go to Tools, then go to the new 'Pages' page 3. Add a new page to display on both the staff client and OPAC. 4. Confirm the URLs in the Page URL column work as expected. 5. Confirm that any pages made for the staff client only do not show in the OPAC, and that any pages made for the OPAC only do not show in the staff client. 6. Confirm that pages do not show on the OPAC main page where news items show. Confirm news items show on the OPAC main page as expected. Sponsored-by: Chartered Accountants Australia and New Zealand Signed-off-by: Thibault Kero <thibault.keromnes@univ-paris8.fr> When I add a custom page for the 'OPAC' or 'Librarian and OPAC' I do not see a news item but I do see the empty news container and the RSS container (.rssnews-container) and words like "RSS feed for Centerville and system-wide library news". Can that be removed if we have no actual news items? Created attachment 138489 [details] [review] Bug 15326: (follow-up) Hide RSS feed if no news items If there are pages but no news items showing on the OPAC, the RSS feed container should not show. Created attachment 138493 [details] [review] Bug 15326: (follow-up) Hide RSS feed if no news items If there are pages but no news items showing on the OPAC, the RSS feed container should not show. Sponsored-by: Horowhenua Libraries Trust Created attachment 139028 [details] [review] Bug 15326: Using AdditionalContents for custom pages on OPAC This page utilises the Additional Contents feature to add custom pages to the staff client and the OPAC in the user's desired language. To test: 1. Apply patch and restart services 2. In the staff client, go to Tools, then go to the new 'Pages' page 3. Add a new page to display on both the staff client and OPAC. 4. Confirm the URLs in the Page URL column work as expected. 5. Confirm that any pages made for the staff client only do not show in the OPAC, and that any pages made for the OPAC only do not show in the staff client. 6. Confirm that pages do not show on the OPAC main page where news items show. Confirm news items show on the OPAC main page as expected. Sponsored-by: Chartered Accountants Australia and New Zealand Signed-off-by: Thibault Kero <thibault.keromnes@univ-paris8.fr> Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Created attachment 139029 [details] [review] Bug 15326: Using AdditionalContents for custom pages on OPAC This page utilises the Additional Contents feature to add custom pages to the staff client and the OPAC in the user's desired language. To test: 1. Apply patch and restart services 2. In the staff client, go to Tools, then go to the new 'Pages' page 3. Add a new page to display on both the staff client and OPAC. 4. Confirm the URLs in the Page URL column work as expected. 5. Confirm that any pages made for the staff client only do not show in the OPAC, and that any pages made for the OPAC only do not show in the staff client. 6. Confirm that pages do not show on the OPAC main page where news items show. Confirm news items show on the OPAC main page as expected. Sponsored-by: Chartered Accountants Australia and New Zealand Signed-off-by: Thibault Kero <thibault.keromnes@univ-paris8.fr> Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Created attachment 139030 [details] [review] Bug 15326: (follow-up) Hide RSS feed if no news items If there are pages but no news items showing on the OPAC, the RSS feed container should not show. Sponsored-by: Horowhenua Libraries Trust Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Created attachment 139031 [details] [review] Bug 15326: (follow-up) Hide RSS feed if no news items If there are pages but no news items showing on the OPAC, the RSS feed container should not show. Sponsored-by: Horowhenua Libraries Trust Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Worked great! Lisette It looks like custom pages in the staff interface require "tools => '*'" permission. I don't know if it's intentional, but I would think that pages should require only basic staff access permission. It would certainly be interesting to include custom permission settings for custom pages, but without such a capability I think general access is a sensible default. One thing which is potentially confusing: It doesn't look like the library choice has any bearing on the behavior of the page? I wonder if users might assume that a page created with a specific library specified will only be viewable to users logged in at that library? Created attachment 139111 [details] [review] Bug 15326: (follow-up) Fix staff permissions and consider library limits To test: 1. Take note of what your logged in library is 2. Add a new page that can be viewed on the OPAC and staff interface. Limit the library to a different library, not your logged in library 3. Use a News item to create a hyperlink to your new page on the OPAC and staff interface. 4. Confirm that a page error shows if you try to view the new page. 5. Edit the new page and set the library to All libraries. 6. Try to go to the new page again and confirm it shows. 7. Log out and log back into the staff interface as a staff user that isn't a superlibrarian 8. Confirm you can view the new page as this user Sponsored-by: Horowhenua Libraries Trust Created attachment 139123 [details] [review] Bug 15326: Using AdditionalContents for custom pages on OPAC This page utilises the Additional Contents feature to add custom pages to the staff client and the OPAC in the user's desired language. To test: 1. Apply patch and restart services 2. In the staff client, go to Tools, then go to the new 'Pages' page 3. Add a new page to display on both the staff client and OPAC. 4. Confirm the URLs in the Page URL column work as expected. 5. Confirm that any pages made for the staff client only do not show in the OPAC, and that any pages made for the OPAC only do not show in the staff client. 6. Confirm that pages do not show on the OPAC main page where news items show. Confirm news items show on the OPAC main page as expected. Sponsored-by: Chartered Accountants Australia and New Zealand Signed-off-by: Thibault Kero <thibault.keromnes@univ-paris8.fr> Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 139124 [details] [review] Bug 15326: (follow-up) Hide RSS feed if no news items If there are pages but no news items showing on the OPAC, the RSS feed container should not show. Sponsored-by: Horowhenua Libraries Trust Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 139125 [details] [review] Bug 15326: (follow-up) Fix staff permissions and consider library limits To test: 1. Take note of what your logged in library is 2. Add a new page that can be viewed on the OPAC and staff interface. Limit the library to a different library, not your logged in library 3. Use a News item to create a hyperlink to your new page on the OPAC and staff interface. 4. Confirm that a page error shows if you try to view the new page. 5. Edit the new page and set the library to All libraries. 6. Try to go to the new page again and confirm it shows. 7. Log out and log back into the staff interface as a staff user that isn't a superlibrarian 8. Confirm you can view the new page as this user Sponsored-by: Horowhenua Libraries Trust Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 139172 [details] [review] Bug 15326: Using AdditionalContents for custom pages on OPAC This page utilises the Additional Contents feature to add custom pages to the staff client and the OPAC in the user's desired language. To test: 1. Apply patch and restart services 2. In the staff client, go to Tools, then go to the new 'Pages' page 3. Add a new page to display on both the staff client and OPAC. 4. Confirm the URLs in the Page URL column work as expected. 5. Confirm that any pages made for the staff client only do not show in the OPAC, and that any pages made for the OPAC only do not show in the staff client. 6. Confirm that pages do not show on the OPAC main page where news items show. Confirm news items show on the OPAC main page as expected. Sponsored-by: Chartered Accountants Australia and New Zealand Signed-off-by: Thibault Kero <thibault.keromnes@univ-paris8.fr> Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 139173 [details] [review] Bug 15326: (follow-up) Hide RSS feed if no news items If there are pages but no news items showing on the OPAC, the RSS feed container should not show. Sponsored-by: Horowhenua Libraries Trust Signed-off-by: Lisette Scheer <lisettePalouse+Koha@gmail.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 139174 [details] [review] Bug 15326: (follow-up) Fix staff permissions and consider library limits To test: 1. Take note of what your logged in library is 2. Add a new page that can be viewed on the OPAC and staff interface. Limit the library to a different library, not your logged in library 3. Use a News item to create a hyperlink to your new page on the OPAC and staff interface. 4. Confirm that a page error shows if you try to view the new page. 5. Edit the new page and set the library to All libraries. 6. Try to go to the new page again and confirm it shows. 7. Log out and log back into the staff interface as a staff user that isn't a superlibrarian 8. Confirm you can view the new page as this user Sponsored-by: Horowhenua Libraries Trust Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> :-D Pushed to master for 22.11. Nice work everyone, thanks! YAY! Over the years this development has been sponsored by Region Halland, Chartered Accountants Australia and New Zealand, and Horowhenua Libraries Trust. I'm sure they'll all be so pleased this has been pushed. Thanks all (In reply to Aleisha Amohia from comment #181) > YAY! Over the years this development has been sponsored by Region Halland, > Chartered Accountants Australia and New Zealand, and Horowhenua Libraries > Trust. I'm sure they'll all be so pleased this has been pushed. Thanks all Yeah, it was out of radar, and I was very happy about pushing it when noticed the QA team approved it. Only the HLT got credited on the patches, though. If possible, please pm me the right names for the sponsors so I add them when the release happens. Thanks and congrats! Great to see this got pushed while making use of the additional contents functionality that wasnt there at the start. We suffer here from a similar issue as on bug 31370 (for the preview button). The URL shown is the one belonging to 'default'. But when you add another langauge, you will create another record (another idnew) with another URL which is not shown on the list view. But in contrast to the content, this id is only visible in the address bar which could be considered *less* user friendly. What I personally would like, is a URL that works for all languages. If the language is found, show it. If it is not found, fall back to default. In that way you would only need one umbrella URL for page and translations. With reference to the previous comment, I think we should consider bug 31383 (In reply to Marcel de Rooy from comment #184) > But in contrast to the content, this id > is only visible in the address bar which could be considered *less* user > friendly. I need to add that is not shown at all actually. But you could add it yourself on the address bar if you know it. (In reply to Marcel de Rooy from comment #184) > What I personally would like, is a URL that works for all languages. If the > language is found, show it. If it is not found, fall back to default. In > that way you would only need one umbrella URL for page and translations. See my proposal on bug 31385 (In reply to Marcel de Rooy from comment #187) > (In reply to Marcel de Rooy from comment #184) > > What I personally would like, is a URL that works for all languages. If the > > language is found, show it. If it is not found, fall back to default. In > > that way you would only need one umbrella URL for page and translations. > > See my proposal on bug 31385 Thanks! Can you take a look, Kyle? For documentation team, might want to look at Bug 7840 Very nice improvement that will not be backported to 22.05.x :) If I create a new page I see: idnew | category | code | location 5 | pages | staff_and_opac_5 | staff_and_opac Why is the code "staff_and_opac_5"? Shouldn't it be "pages_5" instead? (In reply to Jonathan Druart from comment #191) > If I create a new page I see: > > idnew | category | code | location > 5 | pages | staff_and_opac_5 | staff_and_opac > > Why is the code "staff_and_opac_5"? Shouldn't it be "pages_5" instead? It would have been better, yes. But I actually would like to change it to an 'alias' of my choice anyway. There should be several reports in that direction already. |