Bug 24387 - Rename News tool
Summary: Rename News tool
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
: 22663 24766 25001 27451 (view as bug list)
Depends on: 22544
Blocks: 29424 29782 29805 29041 29042 29113 29153 29263 29515 29528 29717 29778 29828
  Show dependency treegraph
 
Reported: 2020-01-09 16:17 UTC by Caroline Cyr La Rose
Modified: 2022-01-08 21:30 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement is renaming the "News" tool to the more generic "Additional contents". It creates two different "categories" of content: "news" and "HTML customizations".
Version(s) released in:
21.11.00


Attachments
Bug 24387: Rename "News" with "Additional contents" (153.83 KB, patch)
2021-07-09 12:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: DBIC Schema changes (8.35 KB, patch)
2021-07-09 12:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: Fix location options when editing HTML custom (736 bytes, patch)
2021-07-09 13:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Dump of dumb data for opac_news to replay the db rev easily (5.74 KB, application/sql)
2021-07-09 13:40 UTC, Jonathan Druart
Details
Bug 24387: Rename "News" with "Additional contents" (154.05 KB, patch)
2021-07-16 07:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: Fix location options when editing HTML custom (736 bytes, patch)
2021-07-16 07:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: DBIC schema changes (8.33 KB, patch)
2021-07-16 07:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: Rename "News" with "Additional contents" (154.02 KB, patch)
2021-07-16 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24387: Fix location options when editing HTML custom (799 bytes, patch)
2021-07-16 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24387: DBIC schema changes (8.39 KB, patch)
2021-07-16 09:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24387: Rename "News" with "Additional contents" (154.09 KB, patch)
2021-08-10 23:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24387: Fix location options when editing HTML custom (854 bytes, patch)
2021-08-10 23:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24387: DBIC schema changes (8.44 KB, patch)
2021-08-10 23:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24387: (QA follow-up) Add missing filters (2.62 KB, patch)
2021-08-10 23:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24387: (QA follow-up) Fix method import issues (1.07 KB, patch)
2021-08-10 23:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24387: Fix sample_news.yml installer file (1.82 KB, patch)
2021-08-11 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: Remove sample_news.for other languages (5.51 KB, patch)
2021-08-11 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: Fix sample_news.yml installer file (1.87 KB, patch)
2021-08-11 15:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24387: Remove sample_news.for other languages (5.56 KB, patch)
2021-08-11 15:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24387: Fix sample_news.yml installer file (1.87 KB, patch)
2021-08-11 21:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: Fix sample_news.yml installer file (1.92 KB, patch)
2021-08-12 13:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24387: Fix one remaining occurrence of KohaNews (1.31 KB, patch)
2021-08-16 13:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: Fix AdditionalContents.t (3.59 KB, patch)
2021-08-16 13:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: Fix Letters/TemplateToolkit.t (2.82 KB, patch)
2021-08-16 13:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: Entries with parent are missing a code (1.38 KB, patch)
2021-09-14 17:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24387: Entries with parent are missing a code (1.43 KB, patch)
2021-09-30 15:41 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Caroline Cyr La Rose 2020-01-09 16:17:31 UTC
Now that the 'News' tool serves a larger purpose than just adding news, we should rename it.

I suggest 'Content' or 'Custom content'. But it's just a suggestion. Feel free to change it to something else.
Comment 1 Owen Leonard 2020-01-09 16:37:00 UTC
Maybe "News and content" ?

It's technically redundant but I worry that people will wonder where News went.
Comment 2 Katrin Fischer 2020-04-05 14:54:30 UTC
*** Bug 25001 has been marked as a duplicate of this bug. ***
Comment 3 David Cook 2020-04-06 01:18:21 UTC
As I mention on https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25001, it seems the CMS functionality will use a heading of "Pages" with a description of "Create and edit additional content pages".

I've asked one librarian and they suggested "Interface customization" or "Home page customization" or something like that. 

Martin suggested "Blocks" or "Content blocks", which I quite like, since it implies that it's chunks of content rather than whole pages (like the CMS), but I don't think librarians will understand. 

Maybe "HTML customization"? But that would only work for tech savvy librarians. Maybe "Web page customization" or "Page customization", but then I worry about the overlap with the CMS functionality. 

That said, maybe we should frame it all with a "Edit content" or "Content" heading and then have subheadings for "Pages" and "Sections"?
Comment 4 Lisette Scheer 2020-04-09 15:20:20 UTC
(In reply to Owen Leonard from comment #1)
> Maybe "News and content" ?
> 
> It's technically redundant but I worry that people will wonder where News
> went.

+1 We still use this for news and I think it would confuse some of our folks who use that feature.
Comment 5 Lucas Gass 2020-04-09 16:13:57 UTC
Some librarians use a lot of News items (both in slips and in the the OPAC ). That makes for a lot of clutter once we start adding the HTML preferences. What are the thoughts of separating these into separate 'Tools'?

-News
-Another section for "HTML customization" or whatever is decided to call it
Comment 6 Lisette Scheer 2020-04-09 16:26:08 UTC
(In reply to Lucas Gass from comment #5)
> Some librarians use a lot of News items (both in slips and in the the OPAC
> ). That makes for a lot of clutter once we start adding the HTML
> preferences. What are the thoughts of separating these into separate 'Tools'?
> 
> -News
> -Another section for "HTML customization" or whatever is decided to call it

This is even better. Especially in consortium where different libraries might all be using the news for different things, it could be very clunky to have all the news + these content fields we are moving into the news.
Comment 7 Ed Veal 2020-12-13 17:05:19 UTC
I support separating news and html customizations.
Comment 8 Séverine Queune 2021-04-06 15:24:31 UTC
(In reply to Lucas Gass from comment #5)
> Some librarians use a lot of News items (both in slips and in the the OPAC
> ). That makes for a lot of clutter once we start adding the HTML
> preferences. What are the thoughts of separating these into separate 'Tools'?
> 
> -News
> -Another section for "HTML customization" or whatever is decided to call it


+1 to separate them !
Comment 9 Michael Adamyk 2021-04-07 21:28:46 UTC
+1 to separating them. News is extremely cluttered now (as a consortium).
Comment 10 Jonathan Druart 2021-04-08 09:49:31 UTC
Please help bug 22544 if you want to see this move forward.
Comment 11 Jonathan Druart 2021-06-25 07:44:27 UTC
I am suggesting:

* Codebase will use "additional_contents", Koha::AdditionalContents

* UI will split:
  Tools > News (same as now)
  Tools > HTML customization

Would that work for everybody?
Comment 12 Katrin Fischer 2021-06-26 11:06:30 UTC
(In reply to Jonathan Druart from comment #11)
> I am suggesting:
> 
> * Codebase will use "additional_contents", Koha::AdditionalContents
> 
> * UI will split:
>   Tools > News (same as now)
>   Tools > HTML customization
> 
> Would that work for everybody?

+1
Comment 13 Jonathan Druart 2021-07-09 12:37:11 UTC
Created attachment 122737 [details] [review]
Bug 24387: Rename "News" with "Additional contents"

One big patch for one big move.

The "News" feature (opac_news) has been hijacked to handle some system
preferences (bug 26050). The goal was to take profit of the UI (editor)
and the ability to translate the value.

Disclaimer: This patch is NOT offering the best implementation but, as
we still don't have bug 24975, it cannot be done now. And no, we don't
want to wait for it to move forward here. This patch is going into the
right direction anyway.

This enhancement is going to rename the "News" with a more genertic
"Additional contents". We have two different "categories" of content:
"news" and "html customizations".

What does it bring?
- A split on the UI for disambigate the two types of content (news and
syspref/html customizations)
- A simplification of the edit form: all languages will be translatable
on the same view (like the "notice templates")
- Ground will be prepared for different types of content (if needed later)
- Staff news can be translated

How was the "News" area working before this patch?
The opac_news DB table contained a (very inconsistent) 'lang' column.
The different values were:
- '' => news to display at the OPAC and staff interfaces
- 'koha' => news for staff only
- 'slip' => news for slip notices
- $lang => news for OPAC only, translated in $lang ('en', 'es-ES', etc.)
- "$location_$lang" => A syspref moved to this "news" area. The syspref
is $location, and is translated in $lang. Eg. OpacLoginInstructions_en,
OpacLoginInstructions_fr-FR, opacheader_es-ES

This patch is improving the DB structure with the following changes:
- renaming 'opac_news' with 'additional_contents'
- new 'category' column
 => 'news' or 'html_customizations'
- new 'location' column
 => For 'news': 'staff_and_opac', 'staff_only', 'slip'
 => For 'html_customizations': the old syspref name (eg. 'OpacLoginInstructions').
- new 'code' column (see later for more info)
- the 'lang' column will only contain the language code ('en', 'es-ES',
etc.). BUT a 'default' entry will ALWAYS exist for fallback behaviour.

We are getting closer to the 'notice template' table structure because
we want to match its UI. The 'code' column will bring us the ability to
group the different 'additional_contents' rows. The code for a given
news will be the same, but the (lang, title, content) will differ.
Examples:
  News 1 will have, for each of the translated versions
  (category, code,    location, branchcode)
  ('news',   'News1', $location, $branchcode||undef)
  And the 3 following columns will differ:
  (title,              content,               lang)
  ('title for news 1', 'content for news 1', 'default')
  ('titulo para 1',    'contenido para 1',   'es-ES')

Note that the "category" is not strictely necessary, but it seems better
to have the ability to split the different content by category/type
easily.

Additional changes:
- Syspref 'NewsToolEditor' is renamed 'AdditionalContentsEditor'
- Koha::NewItem => Koha::AdditionalContent
- Koha::News => Koha::AdditionalContents
- Script and template renamed from koha-news to additional-contents
- Foreign keys have been renamed
- Subpermission edit_news has been renamed edit_additional_contents
- The UI can now be accessed via a "News" or "HTML customizations" link
from the tools module. The related contents will then be displayed (both
categories are now split)

Changes not done here:
- Primary key 'idnew' could be renamed 'id'

Limitations of the upgrade:
News cannot be grouped by a unique code for existing translations.
=> A given news will be now displayed several times on the translated
interface
Any ideas to improve the upgrade behaviour?
We will have to add a warning in the release notes to tell libraries to
review their news.

Test plan:
0. Don't apply the patches
1. Translate the interfaces in some languages
. Create some news for staff and OPAC
. Create some content for different entry of HTML customizations
Note that you are forced to define a 'default'.
Also note that you are only forced to fill the title (not the content).
This is certainly problematic (see FIXME in the code) as sometime only
the content is displayed.
. Play with the interface (edit, delete, filter)
. Go to the different places the news are displayed, and confirm they
are displayed correctly (staff home, opac home, opac rss)
. Create 1+ news for 'slip', check an item out and 'print slip' (from
the circulation page). You must see the news.
. Go to the different places you are expecting the HTML customizations
to be present and confirm that you see them.
. Switch the lang of the interface and confirm that you now see the
content in the translated version
. Generate the templates in another language, don't translate the
content
. Use this language for the interface and confirm that the 'default'
version is displauyed.
Comment 14 Jonathan Druart 2021-07-09 12:37:15 UTC
Created attachment 122738 [details] [review]
Bug 24387: DBIC Schema changes

DBIC update

dbic update
Comment 16 Martin Renvoize 2021-07-09 13:14:37 UTC
I still prefer 'Blocks' or 'Content blocks' but am happy to be overruled there.

However, this isn't working fully for me.. in the HTML Customisations screen if I attempt to edit an existing block the Display locations dropdown only shows the News options and not the Nav blocks etc.

As later change, I'd love to see a more visual display on the blocks page highlighting where the blocks of content will actually appear rather than just a table.. but that's certainly a later enhancement.
Comment 17 Jonathan Druart 2021-07-09 13:28:33 UTC
(In reply to Martin Renvoize from comment #16)
> I still prefer 'Blocks' or 'Content blocks' but am happy to be overruled
> there.

Well, Katrin disagreed on that one, and that's why I've asked here before starting two days ago! :D
Now it will be quite a nightmare to replace all the occurrences (script and template names, DB table, variables, modules...)

> However, this isn't working fully for me.. in the HTML Customisations screen
> if I attempt to edit an existing block the Display locations dropdown only
> shows the News options and not the Nav blocks etc.

Indeed, patch on its way!

> As later change, I'd love to see a more visual display on the blocks page
> highlighting where the blocks of content will actually appear rather than
> just a table.. but that's certainly a later enhancement.

Can you detail?
Comment 18 Jonathan Druart 2021-07-09 13:29:06 UTC
Created attachment 122746 [details] [review]
Bug 24387: Fix location options when editing HTML custom
Comment 19 Jonathan Druart 2021-07-09 13:33:55 UTC
And I finally forgot to write those tests for search_for_display!
Will do next week if I don't forget. It does not prevent testing ;)
Comment 20 Jonathan Druart 2021-07-09 13:40:52 UTC
Created attachment 122748 [details]
Dump of dumb data for opac_news to replay the db rev easily
Comment 21 Jonathan Druart 2021-07-12 08:27:08 UTC
*** Bug 27451 has been marked as a duplicate of this bug. ***
Comment 22 Jonathan Druart 2021-07-12 08:28:30 UTC
*** Bug 22663 has been marked as a duplicate of this bug. ***
Comment 23 Jonathan Druart 2021-07-12 08:29:11 UTC
*** Bug 24766 has been marked as a duplicate of this bug. ***
Comment 24 Jonathan Druart 2021-07-16 07:16:56 UTC
Created attachment 122872 [details] [review]
Bug 24387: Rename "News" with "Additional contents"

One big patch for one big move.

The "News" feature (opac_news) has been hijacked to handle some system
preferences (bug 26050). The goal was to take profit of the UI (editor)
and the ability to translate the value.

Disclaimer: This patch is NOT offering the best implementation but, as
we still don't have bug 24975, it cannot be done now. And no, we don't
want to wait for it to move forward here. This patch is going into the
right direction anyway.

This enhancement is going to rename the "News" with a more genertic
"Additional contents". We have two different "categories" of content:
"news" and "html customizations".

What does it bring?
- A split on the UI for disambigate the two types of content (news and
syspref/html customizations)
- A simplification of the edit form: all languages will be translatable
on the same view (like the "notice templates")
- Ground will be prepared for different types of content (if needed later)
- Staff news can be translated

How was the "News" area working before this patch?
The opac_news DB table contained a (very inconsistent) 'lang' column.
The different values were:
- '' => news to display at the OPAC and staff interfaces
- 'koha' => news for staff only
- 'slip' => news for slip notices
- $lang => news for OPAC only, translated in $lang ('en', 'es-ES', etc.)
- "$location_$lang" => A syspref moved to this "news" area. The syspref
is $location, and is translated in $lang. Eg. OpacLoginInstructions_en,
OpacLoginInstructions_fr-FR, opacheader_es-ES

This patch is improving the DB structure with the following changes:
- renaming 'opac_news' with 'additional_contents'
- new 'category' column
 => 'news' or 'html_customizations'
- new 'location' column
 => For 'news': 'staff_and_opac', 'staff_only', 'slip'
 => For 'html_customizations': the old syspref name (eg. 'OpacLoginInstructions').
- new 'code' column (see later for more info)
- the 'lang' column will only contain the language code ('en', 'es-ES',
etc.). BUT a 'default' entry will ALWAYS exist for fallback behaviour.

We are getting closer to the 'notice template' table structure because
we want to match its UI. The 'code' column will bring us the ability to
group the different 'additional_contents' rows. The code for a given
news will be the same, but the (lang, title, content) will differ.
Examples:
  News 1 will have, for each of the translated versions
  (category, code,    location, branchcode)
  ('news',   'News1', $location, $branchcode||undef)
  And the 3 following columns will differ:
  (title,              content,               lang)
  ('title for news 1', 'content for news 1', 'default')
  ('titulo para 1',    'contenido para 1',   'es-ES')

Note that the "category" is not strictely necessary, but it seems better
to have the ability to split the different content by category/type
easily.

Additional changes:
- Syspref 'NewsToolEditor' is renamed 'AdditionalContentsEditor'
- Koha::NewItem => Koha::AdditionalContent
- Koha::News => Koha::AdditionalContents
- Script and template renamed from koha-news to additional-contents
- Foreign keys have been renamed
- Subpermission edit_news has been renamed edit_additional_contents
- The UI can now be accessed via a "News" or "HTML customizations" link
from the tools module. The related contents will then be displayed (both
categories are now split)

Changes not done here:
- Primary key 'idnew' could be renamed 'id'

Limitations of the upgrade:
News cannot be grouped by a unique code for existing translations.
=> A given news will be now displayed several times on the translated
interface
Any ideas to improve the upgrade behaviour?
We will have to add a warning in the release notes to tell libraries to
review their news.

Test plan:
0. Don't apply the patches
1. Translate the interfaces in some languages
. Create some news for staff and OPAC
. Create some content for different entry of HTML customizations
Note that you are forced to define a 'default'.
Also note that you are only forced to fill the title (not the content).
This is certainly problematic (see FIXME in the code) as sometime only
the content is displayed.
. Play with the interface (edit, delete, filter)
. Go to the different places the news are displayed, and confirm they
are displayed correctly (staff home, opac home, opac rss)
. Create 1+ news for 'slip', check an item out and 'print slip' (from
the circulation page). You must see the news.
. Go to the different places you are expecting the HTML customizations
to be present and confirm that you see them.
. Switch the lang of the interface and confirm that you now see the
content in the translated version
. Generate the templates in another language, don't translate the
content
. Use this language for the interface and confirm that the 'default'
version is displauyed.
Comment 25 Jonathan Druart 2021-07-16 07:17:01 UTC
Created attachment 122873 [details] [review]
Bug 24387: Fix location options when editing HTML custom
Comment 26 Jonathan Druart 2021-07-16 07:17:07 UTC
Created attachment 122874 [details] [review]
Bug 24387: DBIC schema changes
Comment 27 Martin Renvoize 2021-07-16 09:57:04 UTC
Created attachment 122878 [details] [review]
Bug 24387: Rename "News" with "Additional contents"

One big patch for one big move.

The "News" feature (opac_news) has been hijacked to handle some system
preferences (bug 26050). The goal was to take profit of the UI (editor)
and the ability to translate the value.

Disclaimer: This patch is NOT offering the best implementation but, as
we still don't have bug 24975, it cannot be done now. And no, we don't
want to wait for it to move forward here. This patch is going into the
right direction anyway.

This enhancement is going to rename the "News" with a more genertic
"Additional contents". We have two different "categories" of content:
"news" and "html customizations".

What does it bring?
- A split on the UI for disambigate the two types of content (news and
syspref/html customizations)
- A simplification of the edit form: all languages will be translatable
on the same view (like the "notice templates")
- Ground will be prepared for different types of content (if needed later)
- Staff news can be translated

How was the "News" area working before this patch?
The opac_news DB table contained a (very inconsistent) 'lang' column.
The different values were:
- '' => news to display at the OPAC and staff interfaces
- 'koha' => news for staff only
- 'slip' => news for slip notices
- $lang => news for OPAC only, translated in $lang ('en', 'es-ES', etc.)
- "$location_$lang" => A syspref moved to this "news" area. The syspref
is $location, and is translated in $lang. Eg. OpacLoginInstructions_en,
OpacLoginInstructions_fr-FR, opacheader_es-ES

This patch is improving the DB structure with the following changes:
- renaming 'opac_news' with 'additional_contents'
- new 'category' column
 => 'news' or 'html_customizations'
- new 'location' column
 => For 'news': 'staff_and_opac', 'staff_only', 'slip'
 => For 'html_customizations': the old syspref name (eg. 'OpacLoginInstructions').
- new 'code' column (see later for more info)
- the 'lang' column will only contain the language code ('en', 'es-ES',
etc.). BUT a 'default' entry will ALWAYS exist for fallback behaviour.

We are getting closer to the 'notice template' table structure because
we want to match its UI. The 'code' column will bring us the ability to
group the different 'additional_contents' rows. The code for a given
news will be the same, but the (lang, title, content) will differ.
Examples:
  News 1 will have, for each of the translated versions
  (category, code,    location, branchcode)
  ('news',   'News1', $location, $branchcode||undef)
  And the 3 following columns will differ:
  (title,              content,               lang)
  ('title for news 1', 'content for news 1', 'default')
  ('titulo para 1',    'contenido para 1',   'es-ES')

Note that the "category" is not strictely necessary, but it seems better
to have the ability to split the different content by category/type
easily.

Additional changes:
- Syspref 'NewsToolEditor' is renamed 'AdditionalContentsEditor'
- Koha::NewItem => Koha::AdditionalContent
- Koha::News => Koha::AdditionalContents
- Script and template renamed from koha-news to additional-contents
- Foreign keys have been renamed
- Subpermission edit_news has been renamed edit_additional_contents
- The UI can now be accessed via a "News" or "HTML customizations" link
from the tools module. The related contents will then be displayed (both
categories are now split)

Changes not done here:
- Primary key 'idnew' could be renamed 'id'

Limitations of the upgrade:
News cannot be grouped by a unique code for existing translations.
=> A given news will be now displayed several times on the translated
interface
Any ideas to improve the upgrade behaviour?
We will have to add a warning in the release notes to tell libraries to
review their news.

Test plan:
0. Don't apply the patches
1. Translate the interfaces in some languages
. Create some news for staff and OPAC
. Create some content for different entry of HTML customizations
Note that you are forced to define a 'default'.
Also note that you are only forced to fill the title (not the content).
This is certainly problematic (see FIXME in the code) as sometime only
the content is displayed.
. Play with the interface (edit, delete, filter)
. Go to the different places the news are displayed, and confirm they
are displayed correctly (staff home, opac home, opac rss)
. Create 1+ news for 'slip', check an item out and 'print slip' (from
the circulation page). You must see the news.
. Go to the different places you are expecting the HTML customizations
to be present and confirm that you see them.
. Switch the lang of the interface and confirm that you now see the
content in the translated version
. Generate the templates in another language, don't translate the
content
. Use this language for the interface and confirm that the 'default'
version is displauyed.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize 2021-07-16 09:57:09 UTC
Created attachment 122879 [details] [review]
Bug 24387: Fix location options when editing HTML custom

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize 2021-07-16 09:57:13 UTC
Created attachment 122880 [details] [review]
Bug 24387: DBIC schema changes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 30 Martin Renvoize 2021-07-16 10:00:10 UTC
This is a great improvement, the biggest bit to me being the 'default' fallback behaviour.

I can't come up with a nicer way to do the DB update, but I think we can cover ourselves with release notes.

We can certainly continue to iterate here on follow-up bugs.. I'm still keen to split the tables back out for different functions.. but that can certainly come later.

Signing off.
Comment 31 Tomás Cohen Arazi 2021-08-10 23:56:43 UTC
Created attachment 123730 [details] [review]
Bug 24387: Rename "News" with "Additional contents"

One big patch for one big move.

The "News" feature (opac_news) has been hijacked to handle some system
preferences (bug 26050). The goal was to take profit of the UI (editor)
and the ability to translate the value.

Disclaimer: This patch is NOT offering the best implementation but, as
we still don't have bug 24975, it cannot be done now. And no, we don't
want to wait for it to move forward here. This patch is going into the
right direction anyway.

This enhancement is going to rename the "News" with a more genertic
"Additional contents". We have two different "categories" of content:
"news" and "html customizations".

What does it bring?
- A split on the UI for disambigate the two types of content (news and
syspref/html customizations)
- A simplification of the edit form: all languages will be translatable
on the same view (like the "notice templates")
- Ground will be prepared for different types of content (if needed later)
- Staff news can be translated

How was the "News" area working before this patch?
The opac_news DB table contained a (very inconsistent) 'lang' column.
The different values were:
- '' => news to display at the OPAC and staff interfaces
- 'koha' => news for staff only
- 'slip' => news for slip notices
- $lang => news for OPAC only, translated in $lang ('en', 'es-ES', etc.)
- "$location_$lang" => A syspref moved to this "news" area. The syspref
is $location, and is translated in $lang. Eg. OpacLoginInstructions_en,
OpacLoginInstructions_fr-FR, opacheader_es-ES

This patch is improving the DB structure with the following changes:
- renaming 'opac_news' with 'additional_contents'
- new 'category' column
 => 'news' or 'html_customizations'
- new 'location' column
 => For 'news': 'staff_and_opac', 'staff_only', 'slip'
 => For 'html_customizations': the old syspref name (eg. 'OpacLoginInstructions').
- new 'code' column (see later for more info)
- the 'lang' column will only contain the language code ('en', 'es-ES',
etc.). BUT a 'default' entry will ALWAYS exist for fallback behaviour.

We are getting closer to the 'notice template' table structure because
we want to match its UI. The 'code' column will bring us the ability to
group the different 'additional_contents' rows. The code for a given
news will be the same, but the (lang, title, content) will differ.
Examples:
  News 1 will have, for each of the translated versions
  (category, code,    location, branchcode)
  ('news',   'News1', $location, $branchcode||undef)
  And the 3 following columns will differ:
  (title,              content,               lang)
  ('title for news 1', 'content for news 1', 'default')
  ('titulo para 1',    'contenido para 1',   'es-ES')

Note that the "category" is not strictely necessary, but it seems better
to have the ability to split the different content by category/type
easily.

Additional changes:
- Syspref 'NewsToolEditor' is renamed 'AdditionalContentsEditor'
- Koha::NewItem => Koha::AdditionalContent
- Koha::News => Koha::AdditionalContents
- Script and template renamed from koha-news to additional-contents
- Foreign keys have been renamed
- Subpermission edit_news has been renamed edit_additional_contents
- The UI can now be accessed via a "News" or "HTML customizations" link
from the tools module. The related contents will then be displayed (both
categories are now split)

Changes not done here:
- Primary key 'idnew' could be renamed 'id'

Limitations of the upgrade:
News cannot be grouped by a unique code for existing translations.
=> A given news will be now displayed several times on the translated
interface
Any ideas to improve the upgrade behaviour?
We will have to add a warning in the release notes to tell libraries to
review their news.

Test plan:
0. Don't apply the patches
1. Translate the interfaces in some languages
. Create some news for staff and OPAC
. Create some content for different entry of HTML customizations
Note that you are forced to define a 'default'.
Also note that you are only forced to fill the title (not the content).
This is certainly problematic (see FIXME in the code) as sometime only
the content is displayed.
. Play with the interface (edit, delete, filter)
. Go to the different places the news are displayed, and confirm they
are displayed correctly (staff home, opac home, opac rss)
. Create 1+ news for 'slip', check an item out and 'print slip' (from
the circulation page). You must see the news.
. Go to the different places you are expecting the HTML customizations
to be present and confirm that you see them.
. Switch the lang of the interface and confirm that you now see the
content in the translated version
. Generate the templates in another language, don't translate the
content
. Use this language for the interface and confirm that the 'default'
version is displauyed.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 32 Tomás Cohen Arazi 2021-08-10 23:56:51 UTC
Created attachment 123731 [details] [review]
Bug 24387: Fix location options when editing HTML custom

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 33 Tomás Cohen Arazi 2021-08-10 23:56:59 UTC
Created attachment 123732 [details] [review]
Bug 24387: DBIC schema changes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 34 Tomás Cohen Arazi 2021-08-10 23:57:06 UTC
Created attachment 123733 [details] [review]
Bug 24387: (QA follow-up) Add missing filters

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 35 Tomás Cohen Arazi 2021-08-10 23:57:14 UTC
Created attachment 123734 [details] [review]
Bug 24387: (QA follow-up) Fix method import issues

Raised by the QA script, a couple issues carried by pre-17600 patches.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 36 Tomás Cohen Arazi 2021-08-11 11:36:10 UTC
Some DB columns are missing default values (category, code and location):

DBD::mysql::db do failed: Field 'code' doesn't have a default value at /kohadevbox/koha/C4/Installer.pm line 572.
Comment 37 Jonathan Druart 2021-08-11 14:52:09 UTC
Created attachment 123795 [details] [review]
Bug 24387: Fix sample_news.yml installer file
Comment 38 Jonathan Druart 2021-08-11 14:52:14 UTC
Created attachment 123796 [details] [review]
Bug 24387: Remove sample_news.for other languages

We shouldn't continue to update those files as we are going to remove
them soon.
Comment 39 Jonathan Druart 2021-08-11 14:52:59 UTC
(In reply to Tomás Cohen Arazi from comment #36)
> Some DB columns are missing default values (category, code and location):
> 
> DBD::mysql::db do failed: Field 'code' doesn't have a default value at
> /kohadevbox/koha/C4/Installer.pm line 572.

It's the installer file that was missing values for those columns! Thx!
Comment 40 Tomás Cohen Arazi 2021-08-11 15:15:03 UTC
Created attachment 123797 [details] [review]
Bug 24387: Fix sample_news.yml installer file

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 41 Tomás Cohen Arazi 2021-08-11 15:15:18 UTC
Created attachment 123798 [details] [review]
Bug 24387: Remove sample_news.for other languages

We shouldn't continue to update those files as we are going to remove
them soon.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 42 Tomás Cohen Arazi 2021-08-11 15:21:32 UTC
This action will erase all your data. Are you sure? (y/N) y
Running [sudo koha-shell kohadev -p -c 'PERL5LIB=/kohadevbox/koha:/kohadevbox/koha/lib:/kohadevbox/qa-test-tools perl /kohadevbox/misc4dev/populate_db.pl -v --opac-base-url http://kohadev.myDNSname.org:8080 --intranet-base-url http://kohadev-intra.myDNSname.org:8081 --marcflavour MARC21']...
C4::Installer::load_sql returned the following errors while attempting to load /kohadevbox/koha/installer/data/mysql/en/optional/sample_news.yml:
DBD::mysql::db do failed: Field 'code' doesn't have a default value at /kohadevbox/koha/C4/Installer.pm line 572.
Inserting koha db structure...
Comment 43 Jonathan Druart 2021-08-11 16:49:33 UTC
With all the patches attached to the bug report right now:

root@kohadevbox:koha(bug_24387)$ reset_all                                                                                                                              
This action will erase all your data. Are you sure? (y/N) y                                        
Running [sudo koha-shell kohadev -p -c 'PERL5LIB=/kohadevbox/koha:/kohadevbox/koha/lib:/kohadevbox/qa-test-tools perl /kohadevbox/misc4dev/populate_db.pl -v --opac-base-url http://kohadev.myDNSname.org:8080 --intr
anet-base-url http://kohadev-intra.myDNSname.org:8081 --marcflavour MARC21']...                
Inserting koha db structure...                                                                                                                                                                                      
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/sysprefs.sql...                        
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/subtag_registry.sql...       
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/auth_val_cat.sql...                                                                                                                                       
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/message_transport_types.sql...
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/sample_notices_message_attributes.sql...                                           
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/sample_notices_message_transports.sql...
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/keyboard_shortcuts.sql...
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/userflags.sql...                   
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/userpermissions.sql...         
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/audio_alerts.sql...  
Skipping /kohadevbox/koha/installer/data/mysql/account_offset_types.sql                   
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/account_credit_types.sql...
Inserting /kohadevbox/koha/installer/data/mysql/mandatory/account_debit_types.sql...
Inserting /kohadevbox/koha/installer/data/mysql/en/mandatory/auth_values.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/mandatory/class_sources.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/mandatory/sample_frequencies.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/mandatory/sample_notices.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/mandatory/sample_numberpatterns.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/auth_val.yml...     
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/csv_profiles.yml...     
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/marc21_holdings_coded_values.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/marc21_relatorterms.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/parameters.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/patron_atributes.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/patron_categories.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/sample_creator_data.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/sample_itemtypes.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/sample_libraries.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/sample_libraries_holidays.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/sample_news.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/sample_patrons.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/sample_quotes.yml...  
Inserting /kohadevbox/koha/installer/data/mysql/en/optional/sample_z3950_servers.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/marcflavour/marc21/mandatory/authorities_normal_marc21.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/marcflavour/marc21/mandatory/marc21_framework_DEFAULT.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/marcflavour/marc21/optional/marc21_default_matching_rules.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/marcflavour/marc21/optional/marc21_sample_fastadd_framework.yml...
Inserting /kohadevbox/koha/installer/data/mysql/en/marcflavour/marc21/optional/marc21_simple_bib_frameworks.yml...
Setting the MARC flavour on the sysprefs...                                        
Setting Koha version to 21.0600014...         

Can you double-check please?
Comment 44 Tomás Cohen Arazi 2021-08-11 17:40:50 UTC
My setup is KTD, using master-buster and mariadb:10.3
Comment 45 Jonathan Druart 2021-08-11 21:06:59 UTC
Created attachment 123803 [details] [review]
Bug 24387: Fix sample_news.yml installer file
Comment 46 Jonathan Druart 2021-08-11 21:08:09 UTC
(In reply to Tomás Cohen Arazi from comment #44)
> My setup is KTD, using master-buster and mariadb:10.3

Same here but with mariadb 10:1:48. The code was missing from yml, obvious mistake, sorry about that.
Comment 47 Jonathan Druart 2021-08-11 21:14:46 UTC
[
We should docker up "d11" and have a D11 with dbms version brought by D11
We shouldn't have to explicitly tell that we need to up with docker-compose.mariadb_d11.yml
Can this be fixed? is it worth opening a new ktd issue?
]
Comment 48 Tomás Cohen Arazi 2021-08-12 13:21:23 UTC
Created attachment 123811 [details] [review]
Bug 24387: Fix sample_news.yml installer file

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 49 Jonathan Druart 2021-08-16 10:04:24 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 50 Jonathan Druart 2021-08-16 13:19:23 UTC
Created attachment 123854 [details] [review]
Bug 24387: Fix one remaining occurrence of KohaNews
Comment 51 Jonathan Druart 2021-08-16 13:19:28 UTC
Created attachment 123855 [details] [review]
Bug 24387: Fix AdditionalContents.t
Comment 52 Jonathan Druart 2021-08-16 13:19:34 UTC
Created attachment 123856 [details] [review]
Bug 24387: Fix Letters/TemplateToolkit.t

We must cast to date or the comparison does not work
Comment 53 Jonathan Druart 2021-08-16 13:20:53 UTC
Last 3 patches pushed to master.
Comment 54 Jonathan Druart 2021-09-14 17:27:21 UTC
Created attachment 124867 [details] [review]
Bug 24387: Entries with parent are missing a code

We executed the same 2 queries, we want to adjust the code for the
parent and the language specific row.
Comment 55 Nick Clemens 2021-09-30 15:41:10 UTC
Created attachment 125545 [details] [review]
Bug 24387: Entries with parent are missing a code

We executed the same 2 queries, we want to adjust the code for the
parent and the language specific row.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 56 Nick Clemens 2021-09-30 15:41:39 UTC
(In reply to Jonathan Druart from comment #54)
> Created attachment 124867 [details] [review] [review]
> Bug 24387: Entries with parent are missing a code
> 
> We executed the same 2 queries, we want to adjust the code for the
> parent and the language specific row.

(In reply to Nick Clemens from comment #55)
> Created attachment 125545 [details] [review] [review]
> Bug 24387: Entries with parent are missing a code
> 
> We executed the same 2 queries, we want to adjust the code for the
> parent and the language specific row.
> 
> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Read code, did not test, but the change makes logical sense
Comment 57 Jonathan Druart 2021-10-06 08:37:19 UTC
(In reply to Nick Clemens from comment #55)
> Created attachment 125545 [details] [review] [review]
> Bug 24387: Entries with parent are missing a code
> 
> We executed the same 2 queries, we want to adjust the code for the
> parent and the language specific row.
> 
> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Pushed to master.