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

Uhf 8103 sitemap urls #52

Merged
merged 14 commits into from
Feb 28, 2023
Merged

Uhf 8103 sitemap urls #52

merged 14 commits into from
Feb 28, 2023

Conversation

rpnykanen
Copy link
Contributor

@rpnykanen rpnykanen commented Feb 23, 2023

UHF-8103

Enhance simple sitemap url generation in certain cases

What was done

Dynamically set simple_sitemap baseurl config override if module is installed.

How to install

  • Make sure your instance is up and running on dev branch.
  • Make sure the site has simple_sitemap installed & enabled (all 9 instances has it )
  • Run composer require drupal/helfi_proxy:dev-UHF-8103_sitemap_urls
  • Run drush cr

How to test

  • Reload any page after cache clear
  • Run inside container drush php:eval "echo json_encode(\Drupal::service('config.factory')->get('simple_sitemap.settings')->get('base_url'));"
  • The command should print "{project.docker.so}/{langcode}/{site-prefix}"

@codecov-commenter
Copy link

codecov-commenter commented Feb 23, 2023

Codecov Report

Merging #52 (da6239d) into main (467379e) will not change coverage.
The diff coverage is n/a.

❗ Current head da6239d differs from pull request most recent head f737f71. Consider uploading reports for the commit f737f71 to get more accurate results

@@            Coverage Diff            @@
##               main      #52   +/-   ##
=========================================
  Coverage     67.50%   67.50%           
  Complexity       11       11           
=========================================
  Files             2        2           
  Lines            40       40           
=========================================
  Hits             27       27           
  Misses           13       13           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@juho-lehmonen juho-lehmonen self-requested a review February 23, 2023 08:31
Copy link

@juho-lehmonen juho-lehmonen left a comment

Choose a reason for hiding this comment

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

This caused duplicates at least in the Rekry instance:

<url>
  <loc>https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/fi/avoimet-tyopaikat</loc>
  <xhtml:link rel="alternate" hreflang="fi" href="[https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/fi/avoimet-tyopaikat](view-source:https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/fi/avoimet-tyopaikat)"/>
  <xhtml:link rel="alternate" hreflang="sv" href="[https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/sv/lediga-jobb](view-source:https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/sv/lediga-jobb)"/>
  <xhtml:link rel="alternate" hreflang="en" href="[https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/en/open-jobs](view-source:https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/en/open-jobs)"/>
  <lastmod>2023-01-11T13:35:31+02:00</lastmod>
  <changefreq>daily</changefreq>
  <priority>1.0</priority>
 </url>
<url>
  <loc>https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/en/open-jobs/open-jobs/palke-02-5-23</loc>
  <xhtml:link rel="alternate" hreflang="fi" href="[https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/fi/avoimet-tyopaikat/avoimet-tyopaikat/palke-02-5-23](view-source:https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/fi/avoimet-tyopaikat/avoimet-tyopaikat/palke-02-5-23)"/>
  <xhtml:link rel="alternate" hreflang="sv" href="[https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/sv/lediga-jobb/lediga-jobb/palke-02-5-23](view-source:https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/sv/lediga-jobb/lediga-jobb/palke-02-5-23)"/>
  <xhtml:link rel="alternate" hreflang="en" href="[https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/en/open-jobs/open-jobs/palke-02-5-23](view-source:https://helfi-rekry.docker.so/fi/avoimet-tyopaikat/en/open-jobs/open-jobs/palke-02-5-23)"/>
  <lastmod>2023-02-13T08:51:42+02:00</lastmod>
  <priority>0.5</priority>
 </url>

Also happens on the SOTE instance:

<url>
  <loc>https://helfi-sote.docker.so/en/health-and-social-services/fi/sosiaali-ja-terveyspalvelut/terveydenhoito/terveysasemat</loc>
  <xhtml:link rel="alternate" hreflang="fi" href="[https://helfi-sote.docker.so/en/health-and-social-services/fi/sosiaali-ja-terveyspalvelut/terveydenhoito/terveysasemat](view-source:https://helfi-sote.docker.so/en/health-and-social-services/fi/sosiaali-ja-terveyspalvelut/terveydenhoito/terveysasemat)"/>
  <xhtml:link rel="alternate" hreflang="sv" href="[https://helfi-sote.docker.so/en/health-and-social-services/sv/social-och-halsovardstjanster/halsovard/halsostationer](view-source:https://helfi-sote.docker.so/en/health-and-social-services/sv/social-och-halsovardstjanster/halsovard/halsostationer)"/>
  <xhtml:link rel="alternate" hreflang="en" href="[https://helfi-sote.docker.so/en/health-and-social-services/en/health-and-social-services/health-care/health-stations](view-source:https://helfi-sote.docker.so/en/health-and-social-services/en/health-and-social-services/health-care/health-stations)"/>
  <lastmod>2022-12-21T09:44:00+02:00</lastmod>
  <priority>0.5</priority>
 </url>

Copy link
Contributor

@khalima khalima left a comment

Choose a reason for hiding this comment

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

This comment has been deleted

$url =& $link['url'];
$link['url'] = preg_replace('/\/fi\/.*?\//', '/', $url, 1);
$alternateUrls =& $link['alternate_urls'];
foreach ($alternateUrls as $key => &$alternateUrl) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's a duplicated $key variable.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 7 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants