Bug 15326 - Add CMS feature
Summary: Add CMS feature
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Target Milestone: ---
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Whiteboard:
Keywords: Academy
: 12806 (view as bug list)
Depends on:
Blocks: 7840
  Show dependency treegraph
 
Reported: 2015-12-07 14:53 UTC by Martin Persson
Modified: 2019-03-22 06:44 UTC (History)
17 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Large patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 15326: Add CMS feature (42.25 KB, patch)
2015-12-13 21:07 UTC, Martin Persson
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] Add CMS feature (55.50 KB, patch)
2016-12-19 03:50 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: Add CMS feature (42.30 KB, patch)
2017-01-05 01:03 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: Add CMS feature (42.30 KB, patch)
2017-01-05 01:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] Add CMS feature (55.46 KB, patch)
2017-01-05 01:05 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] Add CMS feature (54.71 KB, patch)
2017-01-10 22:02 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: Add CMS feature (42.28 KB, patch)
2017-06-02 04:16 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] Add CMS feature (53.86 KB, patch)
2017-06-02 04:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] Many fixes (15.53 KB, patch)
2017-06-12 23:54 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] More fixes (2.69 KB, patch)
2017-07-10 01:32 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] adding and editing fixes (1.35 KB, patch)
2017-07-10 01:53 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing (3.53 KB, patch)
2017-08-20 23:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: Add CMS feature (42.12 KB, patch)
2018-01-24 01:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] Add CMS feature (54.37 KB, patch)
2018-01-24 01:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] Many fixes (15.53 KB, patch)
2018-01-24 01:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] More fixes (2.69 KB, patch)
2018-01-24 01:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] adding and editing fixes (1.35 KB, patch)
2018-01-24 01:41 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing (3.53 KB, patch)
2018-01-24 01:41 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: Add CMS feature (42.15 KB, patch)
2018-06-25 15:32 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15326: (follow-up) Add CMS feature (62.32 KB, patch)
2018-06-25 15:32 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15326: (follow-up) utf8 to utf8mb4 (3.40 KB, patch)
2018-06-25 15:32 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15326: (follow-up) Move js to footer, use Asset plugin (5.65 KB, patch)
2018-06-25 15:32 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15326: (follow-up) Don't make links to unpublished pages in opac (1.63 KB, patch)
2018-06-25 15:32 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15326: (follow-up) Fix tools menu highlighting (2.39 KB, patch)
2018-06-25 15:32 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15326: Add CMS feature (96.80 KB, patch)
2018-08-28 22:53 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Add CMS feature (62.38 KB, patch)
2018-08-28 22:54 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) utf8 to utf8mb4 (3.40 KB, patch)
2018-08-28 22:54 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Move js to footer, use Asset plugin (5.65 KB, patch)
2018-08-28 22:54 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Don't make links to unpublished pages in opac (1.63 KB, patch)
2018-08-28 22:54 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Fix tools menu highlighting (2.39 KB, patch)
2018-08-28 22:54 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Code review fixes (1.91 KB, patch)
2018-08-28 23:38 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Interface fixes (7.19 KB, patch)
2018-08-29 03:21 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Pages navigation fixes (6.16 KB, patch)
2018-09-24 03:01 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: Add CMS feature (96.82 KB, patch)
2018-12-17 21:16 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Add CMS feature (62.51 KB, patch)
2018-12-17 21:16 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) utf8 to utf8mb4 (3.40 KB, patch)
2018-12-17 21:16 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Move js to footer, use Asset plugin (5.65 KB, patch)
2018-12-17 21:16 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Don't make links to unpublished pages in opac (1.63 KB, patch)
2018-12-17 21:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Fix tools menu highlighting (903 bytes, patch)
2018-12-17 21:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Code review fixes (1.91 KB, patch)
2018-12-17 21:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Interface fixes (7.19 KB, patch)
2018-12-17 21:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Pages navigation fixes (6.16 KB, patch)
2018-12-17 21:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: Add CMS feature (96.97 KB, patch)
2018-12-20 09:22 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 15326: (follow-up) Add CMS feature (62.70 KB, patch)
2018-12-20 09:22 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 15326: (follow-up) utf8 to utf8mb4 (3.51 KB, patch)
2018-12-20 09:22 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 15326: (follow-up) Move js to footer, use Asset plugin (5.75 KB, patch)
2018-12-20 09:22 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 15326: (follow-up) Don't make links to unpublished pages in opac (1.74 KB, patch)
2018-12-20 09:23 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 15326: (follow-up) Fix tools menu highlighting (1007 bytes, patch)
2018-12-20 09:23 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 15326: (follow-up) Code review fixes (2.01 KB, patch)
2018-12-20 09:23 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 15326: (follow-up) Interface fixes (7.31 KB, patch)
2018-12-20 09:23 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 15326: (follow-up) Pages navigation fixes (6.29 KB, patch)
2018-12-20 09:23 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 15326: Add CMS feature (96.87 KB, patch)
2019-02-03 23:37 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Add CMS feature (58.97 KB, patch)
2019-02-03 23:37 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) utf8 to utf8mb4 (3.49 KB, patch)
2019-02-03 23:37 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Move js to footer, use Asset plugin (5.74 KB, patch)
2019-02-03 23:37 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Don't make links to unpublished pages in opac (1.73 KB, patch)
2019-02-03 23:37 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Fix tools menu highlighting (1000 bytes, patch)
2019-02-03 23:37 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Code review fixes (2.00 KB, patch)
2019-02-03 23:37 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Interface fixes (7.29 KB, patch)
2019-02-03 23:38 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: (follow-up) Pages navigation fixes (6.25 KB, patch)
2019-02-03 23:38 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15326: Schema changes (4.16 KB, patch)
2019-02-03 23:39 UTC, Aleisha Amohia
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Persson 2015-12-07 14:53:42 UTC
As part of the sponsored project at Region Halland in Sweden we have added simple CMS functionality to Koha. This allows a library to add content pages without having to do any manual work. The content of the pages can be edited from the Staff client in WYSIWYG mode and navigation links are automatically added to OPAC and the staff client. Pages can be specific to branches, languages or OPAC and/or the staff interface. Several levels of pages are supported.
Comment 1 Martin Persson 2015-12-13 21:07:36 UTC
Created attachment 45621 [details] [review]
Bug 15326: Add CMS feature

INCOMPLETE - For evaluation purposes only!
Comment 2 Aleisha Amohia 2016-12-19 03:50:08 UTC
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
Comment 3 Marcel de Rooy 2016-12-19 07:50:02 UTC
Applying: Bug 15326: Add CMS feature
fatal: sha1 information is lacking or useless (Koha/CmsPage.pm).

We seem to miss patches here?
Comment 4 Viktor Sarge 2016-12-28 07:57:22 UTC
Tried it on a freshly installed kohadevbox with jessie with the same result as Marcel.
Comment 5 Aleisha Amohia 2017-01-05 01:03:51 UTC
Created attachment 58607 [details] [review]
Bug 15326: Add CMS feature

INCOMPLETE - For evaluation purposes only!
Comment 6 Aleisha Amohia 2017-01-05 01:04:58 UTC
Created attachment 58608 [details] [review]
Bug 15326: Add CMS feature

INCOMPLETE - For evaluation purposes only!
Comment 7 Aleisha Amohia 2017-01-05 01:05:05 UTC
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
Comment 8 Aleisha Amohia 2017-01-05 01:06:31 UTC
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.
Comment 9 Aleisha Amohia 2017-01-10 22:02:38 UTC
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
Comment 10 Marcel de Rooy 2017-05-17 14:52:28 UTC
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.
Comment 11 Aleisha Amohia 2017-06-02 04:16:58 UTC
Created attachment 63904 [details] [review]
Bug 15326: Add CMS feature

INCOMPLETE - For evaluation purposes only!
Comment 12 Aleisha Amohia 2017-06-02 04:17:05 UTC
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
Comment 13 Aleisha Amohia 2017-06-02 04:17:53 UTC
Merge conflicts fixed, ready for testing
Comment 14 Josef Moravec 2017-06-06 10:28:16 UTC
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)
Comment 15 Aleisha Amohia 2017-06-12 23:54:03 UTC
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
Comment 16 Josef Moravec 2017-06-20 17:44:16 UTC
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
Comment 17 Aleisha Amohia 2017-07-10 01:32:17 UTC
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
Comment 18 Aleisha Amohia 2017-07-10 01:53:03 UTC
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.
Comment 19 Josef Moravec 2017-07-10 11:07:21 UTC
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...
Comment 20 Aleisha Amohia 2017-08-20 23:17:20 UTC
Created attachment 66255 [details] [review]
Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing

Fixes issues from Comment 19, please test
Comment 21 Hugo Agud 2017-08-22 09:21:48 UTC
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?
Comment 22 Caroline Cyr La Rose 2017-09-22 21:16:32 UTC
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.
Comment 23 Aleisha Amohia 2018-01-24 01:40:35 UTC
Created attachment 70866 [details] [review]
Bug 15326: Add CMS feature

INCOMPLETE - For evaluation purposes only!
Comment 24 Aleisha Amohia 2018-01-24 01:40:42 UTC
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
Comment 25 Aleisha Amohia 2018-01-24 01:40:49 UTC
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
Comment 26 Aleisha Amohia 2018-01-24 01:40:56 UTC
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
Comment 27 Aleisha Amohia 2018-01-24 01:41:02 UTC
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.
Comment 28 Aleisha Amohia 2018-01-24 01:41:08 UTC
Created attachment 70871 [details] [review]
Bug 15326: [FOLLOW-UP] Fixing library field in form, opac publishing

Fixes issues from Comment 19, please test
Comment 29 Josef Moravec 2018-06-25 15:32:03 UTC
Created attachment 76387 [details] [review]
Bug 15326: Add CMS feature

INCOMPLETE - For evaluation purposes only!

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 30 Josef Moravec 2018-06-25 15:32:08 UTC
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>
Comment 31 Josef Moravec 2018-06-25 15:32:13 UTC
Created attachment 76389 [details] [review]
Bug 15326: (follow-up) utf8 to utf8mb4

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 32 Josef Moravec 2018-06-25 15:32:18 UTC
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>
Comment 33 Josef Moravec 2018-06-25 15:32:23 UTC
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>
Comment 34 Josef Moravec 2018-06-25 15:32:28 UTC
Created attachment 76392 [details] [review]
Bug 15326: (follow-up) Fix tools menu highlighting

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 35 Josef Moravec 2018-06-25 15:42:23 UTC
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.
Comment 36 Katrin Fischer 2018-07-08 12:04:05 UTC
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!
Comment 37 Aleisha Amohia 2018-08-28 22:53:51 UTC
Created attachment 78232 [details] [review]
Bug 15326: Add CMS feature

INCOMPLETE - For evaluation purposes only!

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 38 Aleisha Amohia 2018-08-28 22:54:00 UTC
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>
Comment 39 Aleisha Amohia 2018-08-28 22:54:08 UTC
Created attachment 78234 [details] [review]
Bug 15326: (follow-up) utf8 to utf8mb4

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 40 Aleisha Amohia 2018-08-28 22:54:15 UTC
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>
Comment 41 Aleisha Amohia 2018-08-28 22:54:22 UTC
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>
Comment 42 Aleisha Amohia 2018-08-28 22:54:29 UTC
Created attachment 78237 [details] [review]
Bug 15326: (follow-up) Fix tools menu highlighting

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 43 Aleisha Amohia 2018-08-28 22:55:00 UTC
Rebased, working on things in Comment 36
Comment 44 Aleisha Amohia 2018-08-28 23:38:25 UTC
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)
Comment 45 Aleisha Amohia 2018-08-29 03:21:31 UTC
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)
Comment 46 Aleisha Amohia 2018-09-24 03:01:21 UTC
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.
Comment 47 Aleisha Amohia 2018-12-17 21:16:32 UTC
Created attachment 83318 [details] [review]
Bug 15326: Add CMS feature

INCOMPLETE - For evaluation purposes only!

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 48 Aleisha Amohia 2018-12-17 21:16:40 UTC
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>
Comment 49 Aleisha Amohia 2018-12-17 21:16:47 UTC
Created attachment 83320 [details] [review]
Bug 15326: (follow-up) utf8 to utf8mb4

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 50 Aleisha Amohia 2018-12-17 21:16:55 UTC
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>
Comment 51 Aleisha Amohia 2018-12-17 21:17:02 UTC
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>
Comment 52 Aleisha Amohia 2018-12-17 21:17:19 UTC
Created attachment 83323 [details] [review]
Bug 15326: (follow-up) Fix tools menu highlighting

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 53 Aleisha Amohia 2018-12-17 21:17:29 UTC
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)
Comment 54 Aleisha Amohia 2018-12-17 21:17:36 UTC
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)
Comment 55 Aleisha Amohia 2018-12-17 21:17:43 UTC
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.
Comment 56 Aleisha Amohia 2018-12-17 21:18:38 UTC
This has been rebased and can be tested. Navigation could still use work.
Comment 57 Michal Denar 2018-12-20 09:22:22 UTC
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>
Comment 58 Michal Denar 2018-12-20 09:22:32 UTC
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>
Comment 59 Michal Denar 2018-12-20 09:22:40 UTC
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>
Comment 60 Michal Denar 2018-12-20 09:22:50 UTC
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>
Comment 61 Michal Denar 2018-12-20 09:23:02 UTC
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>
Comment 62 Michal Denar 2018-12-20 09:23:13 UTC
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>
Comment 63 Michal Denar 2018-12-20 09:23:32 UTC
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>
Comment 64 Michal Denar 2018-12-20 09:23:44 UTC
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>
Comment 65 Michal Denar 2018-12-20 09:23:55 UTC
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>
Comment 66 Katrin Fischer 2019-02-02 20:45:33 UTC
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
Comment 67 Aleisha Amohia 2019-02-03 23:37:14 UTC
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>
Comment 68 Aleisha Amohia 2019-02-03 23:37:24 UTC
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>
Comment 69 Aleisha Amohia 2019-02-03 23:37:31 UTC
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>
Comment 70 Aleisha Amohia 2019-02-03 23:37:37 UTC
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>
Comment 71 Aleisha Amohia 2019-02-03 23:37:45 UTC
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>
Comment 72 Aleisha Amohia 2019-02-03 23:37:52 UTC
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>
Comment 73 Aleisha Amohia 2019-02-03 23:37:59 UTC
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>
Comment 74 Aleisha Amohia 2019-02-03 23:38:06 UTC
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>
Comment 75 Aleisha Amohia 2019-02-03 23:38:13 UTC
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>
Comment 76 Aleisha Amohia 2019-02-03 23:39:22 UTC
Created attachment 84691 [details] [review]
Bug 15326: Schema changes
Comment 77 Aleisha Amohia 2019-02-03 23:39:52 UTC
(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
Comment 78 Katrin Fischer 2019-02-04 06:50:41 UTC
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)
Comment 79 Aleisha Amohia 2019-02-04 19:39:59 UTC
(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!
Comment 80 Katrin Fischer 2019-03-12 00:07:29 UTC
*** Bug 12806 has been marked as a duplicate of this bug. ***
Comment 81 Marcel de Rooy 2019-03-22 06:44:24 UTC
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