OpenDRR / riskprofiler

Web Application to Support Disaster Resilience / Application web pour soutenir la résilience aux catastrophes
MIT License
10 stars 7 forks source link

Carousel mismatch between English and French with different number of slides #118

Open anthonyfok opened 1 year ago

anthonyfok commented 1 year ago

Thanks again to @plesueur for noticing this discrepancy during review:

there is a 'scrolling' quick links bar near the bottom of the landing page, and the count of pages which are linked don't match between the english and french.

image

phil-evans commented 1 year ago

looking at the revisions, 2 of the pages in the french template were removed on march 29th - so that carousel has 7 pages set in english and only 5 in french. can you clarify which pages should actually be shown there?

plesueur commented 1 year ago

Thanks for looking into this, Phil.

The list of links to include on the carousel should be: • Understand the project (links to 'General Information' page) • Explore training materials (links to 'Training Materials' page) • Read FAQs (links to 'Frequently Asked Questions' page) • See glossary (links to 'Glossary' page) • Additional docs (links to 'Additional Documentation' page) • Taking action (links to 'Taking Action' page) • Contact us (links to 'Contact Us page).

Looks like the carousel on the french page is missing 'Understand the Project' and 'Contact Us'

anthonyfok commented 1 year ago

@phil-evans Thank you for looking into this!

@plesueur Thanks! I was making the same list (kind of), but you beat me to it! (Awesome!)

  1. /learn-more/
  2. /learn-more/training-materials/
  3. /frequently-asked-questions/
  4. /learn-more/glossary/
  5. /learn-more/additional-documentation/
  6. /learn-more/taking-action/
  7. /contact/
English French
/learn-more/ /fr/en-savoir-plus/
/learn-more/training-materials/ /fr/en-savoir-plus/documents-de-formation/
/frequently-asked-questions/ /fr/en-savoir-plus/foire-aux-questions/
/learn-more/glossary/ /fr/en-savoir-plus/glossary/
TODO: correct path to /fr/en-savoir-plus/glossaire/
/learn-more/additional-documentation/ /fr/en-savoir-plus/documentation-supplementaire/
/learn-more/taking-action/ /fr/en-savoir-plus/passer-a-l-action/
/contact/ /fr/contactez-nous/
anthonyfok commented 1 year ago

Update: It took me quite a while to find where the “carousel” may be configured. This seems to be the place:

(I am experimenting on my local computer so as not to break things at H7’s development server.)

In English, we have:

image

In French, we currently have:

image

Strange, these are listed in English, but if I remove them and add them again, they are all in French:

image

And click Update, and it should work... But wait! This is very fragile, and appears broken once saved:

image

Fortunately, I was able to restore to an older revision and have it show the carousel cards again, but apparently I cannot modify

I'd better be careful, but interesting to explore nonetheless.

Uh oh, seems to be affecting English too. Simply clicking on [Update] without changing anything would lead to this:

image

@phil-evans, are you seeing this on your end too? Any ideas? Many thanks!

anthonyfok commented 1 year ago

The "Output debug info" is very helpful.

SELECT   wp_posts.*
  FROM wp_posts  LEFT  JOIN wp_icl_translations wpml_translations
  ON wp_posts.ID = wpml_translations.element_id
  AND wpml_translations.element_type = CONCAT('post_', wp_posts.post_type)
  WHERE 1=1  AND wp_posts.ID IN (35,920,922,924,926,928,37)
    AND wp_posts.post_type IN ('post', 'page', 'attachment', 'scenario', 'training', 'indicator', 'community', 'template')
    AND ((wp_posts.post_status = 'inherit'))
    AND ( ( ( wpml_translations.language_code = 'en' OR 0 )
        AND wp_posts.post_type  IN ('post','page','attachment','wp_block','wp_template','wp_template_part','wp_navigation','scenario','training','indicator','community','template','layout' )  )
      OR wp_posts.post_type  NOT  IN ('post','page','attachment','wp_block','wp_template','wp_template_part','wp_navigation','scenario','training','indicator','community','template','layout' )  )
  ORDER BY wp_posts.post_title ASC

Testing it with mysql, I think I found the culprit: (wp_posts.post_status = 'inherit'), but why is it there? What sets it?

Without it, all 7 posts are found, and they all have the post_status of publish rather than inherit. Hmm... (to be continued)

anthonyfok commented 1 year ago

Testing it with mysql, I think I found the culprit: (wp_posts.post_status = 'inherit'), but why is it there? What sets it?

Found it, somewhat by luck; noticed the carousel is in the fw-parent framework, but didn't find anything in carousel PHP files, tried git grep "'inherit'" and found site/assets/themes/fw-parent/resources/vendor/pe-acf-query/acf-query.php has this:

            // if searching for attachments, 
            // post status needs to be set to 'inherit'

            if ( in_array ( 'attachment', $new_query['args']['post_type'] ) ) {
                $new_query['args']['post_status'] = 'inherit';
            }

Commenting out $new_query['args']['post_status'] = 'inherit';, the carousel cards show again, though they got sorted alphabetically which differs from what it was before.

Looks like there has been some code changes to the framework (since around October 2022?) which might have caused a regression in RiskProfiler. I wish I could diagnose more, but that's beyond my WordPress ability. (I could try git bisecting comparing old versions, but I'd better go back to fixing other translation issues.

@phil-evans, when you have time, could you please help us look into it? Many thanks!

anthonyfok commented 1 year ago

For comparison:


English (from a_V1_Main Landing Page_v2_text.docx):

Learn More

New to RiskProfiler or need more information? Resources on the site can help you understand the information provided and its appropriate use.


French (from front-done-10687562_005_FR_a_V1_Main Landing Page_v2_text):

En savoir plus

Nouvel utilisateur de RiskProfiler ou besoin de plus d’information? Les ressources du site peuvent vous aider à comprendre l’information fournie et la façon de l’utiliser.


The current French pages were probably translated again by another helper? They differ from the above. Maybe that's OK.

The most important thing is to add the missing cards, which are apparently "Understand the project / Comprendre le projet" and "Contact Us / Nous joindre".

anthonyfok commented 1 year ago

Temporarily fixed on www.riskprofiler.ca by hand-editing the generated static fr/index.html file, uploading to S3 and invalidating CloudFront cache.