Problem/Motivation

The articles imported during the installation of the demo_umami profile are held in a csv file, with one article per line.
This makes making and reviewing changes to the articles very difficult, as git diff will just show you that the entire line has changed.

Proposed resolution

Move the article body into their own text file. Reference the name of the file in the csv where we currently have the entire instruction text.

Remaining tasks

All the things.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#12 patch-test.png255.8 KBcehfisher
#10 carrots_without_patch-fullpage-export.png1011.03 KBeli-t
#10 carrots_with_patch-fullpage-export.png1011.03 KBeli-t
#10 mushrooms_with_patch-fullpage-export.png1.22 MBeli-t
#10 mushrooms_without_patch-fullpage-export.png1.22 MBeli-t
#10 supermarket_with_patch-fullpage-export.png1.34 MBeli-t
#10 supermarket_without_patch-fullpage-export.png1.34 MBeli-t
#10 herbs_with_patch-fullpage-export.png1.33 MBeli-t
#10 herbs_without_patch-fullpage-export.png1.33 MBeli-t
#10 articles_with_patch-fullpage-export.png831.27 KBeli-t
#10 articles_without_patch-fullpage-export.png831.27 KBeli-t
#9 interdiff_2941837_7_9.txt785 byteseli-t
#9 2941837_9_add_test_for_content_being_imported.patch29.18 KBeli-t
#7 interdiff_2941837_4_7_interdiff.txt1.44 KBeli-t
#7 2941837_7_add_test_for_content_being_imported.patch28.25 KBeli-t
#4 2941837_1_4_interdiff.txt13.27 KBeli-t
#4 2941837_4_actually_import_articles.patch26.58 KBeli-t
#2 2941837_1_article_body_out_of_csv.patch12.96 KBkjay

Comments

kjay created an issue. See original summary.

kjay’s picture

kjay commented
StatusFileSize
new 2941837_1_article_body_out_of_csv.patch12.96 KB

Work in progress. Based on @eli-t's work in #2940362, this patch contains the existing articles as html files in a sub-directory.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented

We should definitely do this.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Issue tags: +Out of the Box Initiative, +Needs tests
StatusFileSize
new 2941837_4_actually_import_articles.patch26.58 KB
new 2941837_1_4_interdiff.txt13.27 KB

Patch to actually import the articles.

Still needs test for htaccess preventing the html being loaded.

Have run this and can see articles being imported that look ok, but cannot currently A/B test against 8.6.x as simplytest.me can't currently build it.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Active » Postponed

Further changes were requested of #2940362: Move recipe instruction out of csv file in to their own text file., so postponing this until that lands

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Postponed » Needs work
eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Needs work » Needs review
StatusFileSize
new 2941837_7_add_test_for_content_being_imported.patch28.25 KB
new interdiff_2941837_4_7_interdiff.txt1.44 KB

Test added to prove the articles are actually loaded on install.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Needs review » Needs work

Still need one more test to prove the html files containing the article body texts are not accessible

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 2941837_9_add_test_for_content_being_imported.patch29.18 KB
new interdiff_2941837_7_9.txt785 bytes

Final test implemented. This is now ready for review.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Issue summary: View changes
StatusFileSize
new articles_without_patch-fullpage-export.png831.27 KB
new articles_with_patch-fullpage-export.png831.27 KB
new herbs_without_patch-fullpage-export.png1.33 MB
new herbs_with_patch-fullpage-export.png1.33 MB
new supermarket_without_patch-fullpage-export.png1.34 MB
new supermarket_with_patch-fullpage-export.png1.34 MB
new mushrooms_without_patch-fullpage-export.png1.22 MB
new mushrooms_with_patch-fullpage-export.png1.22 MB
new carrots_with_patch-fullpage-export.png1011.03 KB
new carrots_without_patch-fullpage-export.png1011.03 KB

Screenshots of article listing and pages for A/B reviewing:

Articles overview before patch

Articles overview after patch

Herbs without

Herbs with

Supermarket without

Supermarket with

Mushrooms without

Mushrooms with

Carrots without

Carrots with

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Issue tags: +Umami stable blocker
cehfisher’s picture

cehfisher commented
StatusFileSize
new patch-test.png255.8 KB

Patch in #9 tested and applied cleanly to local Umami install since SimplyTest.me seems to be having issues with 8.6.x currently. Looks good!

markconroy’s picture

markconroy commented
Status: Needs review » Reviewed & tested by the community

This one is ripe for RTBC

The last submitted patch, 2: 2941837_1_article_body_out_of_csv.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 6eee27d and pushed to 8.6.x. Thanks!

  • alexpott committed 6eee27d on 8.6.x 
    Issue #2941837 by Eli-T, kjay, cehfisher: Move article body out of csv...

  • alexpott committed 8c0a00b on 8.5.x 
    Issue #2941837 by Eli-T, kjay, cehfisher: Move article body out of csv...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.