Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: add integration tests that check the HTML content of different types of pages + fix issues #10043

Merged
merged 44 commits into from
Apr 3, 2024

Conversation

stephanegigandet
Copy link
Contributor

@stephanegigandet stephanegigandet commented Mar 28, 2024

This PR merges the #10023 PR with the main branch, and made corrections that were spotted by the new integration tests that compare the generated HTML:

  • fixed missing language code lc in display_error pages
  • fixed JS issues that were spotted by Sonar Cloud in the expected test results for the generated HTML
  • removed useless select2 entries that were generated but not used any more (spotted by Sonar Cloud)
  • made the genererated HTML / JSON always the same by sorting arrays and keys in hashes
  • removed Old Church Slavonic language that had the same language codes than Church Slavonic (made generated HTML different)
  • simplified the code to generate JSON (e.g. JSON that was converted to UTF8 binary and then converted back to Perl Unicode just after)

We should be able to catch more types of errors with those new tests going forward.

@stephanegigandet stephanegigandet requested a review from a team as a code owner March 28, 2024 11:30
@github-actions github-actions bot added 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies Product Page API WRITE WRITE API to allow sending product info and image labels Mar 28, 2024
Copy link
Member

@teolemon teolemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this PR going to break hackish parsing done mobile side on the assumption that error messages are in English ?

Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great.

I'm a bit sceptic that we don't remove spaces in html (reformat html to a well defined format) because it will induce changes on very basic changes in the template, but it's ok.

We could also consider adding something like https://metacpan.org/pod/Test::HTML::Tidy (or https://metacpan.org/pod/HTML::T5 ?)

lib/ProductOpener/Test.pm Outdated Show resolved Hide resolved
Comment on lines 4274 to 4278
# We need also to remove the canonical URL from the request so that it does not get displayed in the error page
delete $request_ref->{canon_url};
delete $request_ref->{canon_rel_url};
delete $request_ref->{url};
delete $request_ref->{current_link};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shan't we do it in display_error_and_exit ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, I'll move it.

@stephanegigandet
Copy link
Contributor Author

isn't this PR going to break hackish parsing done mobile side on the assumption that error messages are in English ?

Error messages are not in English. e.g. in prod: https://fr.openfoodfacts.org/fdsfdsfsd/FSDQfsqfdsqfedsq/fdsfqsfd

This PR repairs display_error() that was broken by #10008 because we did not have such HTML tests.

You can see the issue on staging right now: view-source:https://fr.openfoodfacts.net/fdsfdsfsd/FSDQfsqfdsqfedsq/fdsfqsfd : it's missing the language code in some generated HTML, like "newsletter-" : <a href="https://link.openfoodfacts.org/newsletter-">S'abonner à notre newsletter</a>

stephanegigandet and others added 2 commits April 2, 2024 17:01
Co-authored-by: Alex Garel <alex@openfoodfacts.org>
@stephanegigandet stephanegigandet enabled auto-merge (squash) April 2, 2024 15:12
@github-actions github-actions bot added the 💥 Merge Conflicts 💥 Merge Conflicts label Apr 2, 2024
@github-actions github-actions bot added the 💥 Merge Conflicts 💥 Merge Conflicts label Apr 3, 2024
@stephanegigandet stephanegigandet enabled auto-merge (squash) April 3, 2024 14:29
Copy link

sonarqubecloud bot commented Apr 3, 2024

Quality Gate Passed Quality Gate passed

Issues
16 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@stephanegigandet stephanegigandet merged commit 95daf63 into main Apr 3, 2024
12 checks passed
@stephanegigandet stephanegigandet deleted the main-html branch April 3, 2024 15:03
john-gom pushed a commit that referenced this pull request May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API v3 API WRITE WRITE API to allow sending product info and image dependencies Pull requests that update a dependency file Display file import 🖼️ Images 🥗 Ingredients 💥 Merge Conflicts 💥 Merge Conflicts Minion 👮 Moderation multilingual products 🔐 Password 🏭 Producers Platform https://wiki.openfoodfacts.org/Platform_for_producers product history We have kept 10 years of product revisions. This is useful to monitor edits & product improvements Product Page Tags 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. 🧪 tests 👥 Users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants