IFRCGo / go-frontend

MIT License
21 stars 5 forks source link

Country pages - GO weblinks to IFRC.org don't support multi-word country names #922

Closed nanometrenat closed 4 years ago

nanometrenat commented 4 years ago

Issue

Weblinks to IFRC.org country pages were added via #776. On the whole these are great, but country names with two-or-more words (e.g. Solomon Islands, Democratic People's Republic of Korea, Federated States of Micronesia, New Zealand etc.) currently don't link correctly, as the URL on GO has spaces in it.

Steps to reproduce

Go to GO Country page e.g. Solomon Islands https://go.ifrc.org/countries/156. The link at the bottom (labelled Solomon Islands on IFRC.org) goes to a URL of https://www.ifrc.org/en/news-and-media/news-stories/asia-pacific/solomon%20islands/ (which returns a PageNotFound screen)

Similarly the FSM country page https://go.ifrc.org/countries/195 links to https://www.ifrc.org/en/news-and-media/news-stories/asia-pacific/micronesia,%20federated%20states%20of/ (which returns a PageNotFound screen)

The DPRK country page https://go.ifrc.org/countries/193#operations currently links to https://www.ifrc.org/en/news-and-media/news-stories/asia-pacific/democratic%20people's%20republic%20of%20korea/ (which returns a PageNotFound screen)

Expected behavior

We need to fix these so they go to correct links. As far as I can tell from my clicks around, the simplest business rules to change the current GO ifrc.org URLs that should fix the vast majority of these broken links are:

e.g. correct links for the first two examples above would be https://www.ifrc.org/en/news-and-media/news-stories/asia-pacific/solomon-islands/ https://www.ifrc.org/en/news-and-media/news-stories/asia-pacific/micronesia-federated-states-of

The full fix for 100% correct links

NOTE: the above business rule changes would improve things a lot but would not fix ALL of the broken links however, as some of the IFRC web pages use a different word order to what's used on GO - e.g. DPRK on GO is DPRK vs on IFRC.org it's KDPR Real IFRC.org link is https://www.ifrc.org/en/news-and-media/news-stories/asia-pacific/korea-democratic-peoples-republic-of/ and in other cases it's not solely a word order thing e.g. For Iran the GO page links to https://www.ifrc.org/en/news-and-media/news-stories/middle-east-and-north-africa/iran,%20islamic%20republic%20of/ whereas the actual link on IFRC.org is https://www.ifrc.org/en/news-and-media/news-stories/middle-east-and-north-africa/iran/ (one word country name rather than the full country name)

Presumably a separate mapping table would be needed if we are to explicitly link all countries (or to have a manual override for cases where IFRC.org web page conventions are not the same as GO). This is similar to the country discussion in #415

@lukecaley can you please confirm how you would like to approach this? Thx

nanometrenat commented 4 years ago

For the avoidance of doubt, this is not an urgent request, just one for the list - but I don't have the power to do labels as yet...

batpad commented 4 years ago

Thanks for the detailed issue @nanometrenat . This seems clear, will add it to the project board and this should be an easy fix to prioritize.

LukeCaley commented 4 years ago

Thanks @nanometrenat. We should be able to do this as a one-off process verifying against the IFRC.org site so a simple process to download a master table and amend URLs is required.

nanometrenat commented 4 years ago

As per @LukeCaley comment above we need to add an extra field in the countries table for IFRC web URL, and we will then manually populate (rather than constructing the IFRC.org URL programmatically).

Then can update the countries page to pull from that table once populated.

GergiH commented 4 years ago

@nanometrenat Right now the regions in the path are pretty hardcoded as well, and while looking around the site /europe-and-central-asia is a valid part of the path, in some cases it wasn't working with some countries for ex.:

Do we want the field to allow one to specify the whole URL, or just the region/country part of the path?

At the moment they are pretty hard-coded via slugs: https://github.com/IFRCGo/go-frontend/blob/6217edd140a5c32cb06d905a0804f8e35097ed60/app/assets/scripts/utils/region-constants.js#L212-L243

nanometrenat commented 4 years ago

Thanks - I think if we're specifying any part of the path we may as well specify the whole path... that way we can quickly change things up if/when the IFRC.org website gets restructured... Cheers :)

teklal commented 4 years ago

@GregoryHorvath when will the frontend PR be deployed to staging (or a separate url) for testing? I see the url field has been added in the backend but won't be able to fully test this until frontend is deployed too.

GergiH commented 4 years ago

@GregoryHorvath when will the frontend PR be deployed to staging (or a separate url) for testing? I see the url field has been added in the backend but won't be able to fully test this until frontend is deployed too.

@teklal Oh right sorry for that, I forgot I assigned that review to Sanjay but he's busier these days than ever so I just pulled that into develop so it should be on staging in ~5 mins I believe (or maybe it already is while I finish this comment).

teklal commented 4 years ago

@GregoryHorvath So the thing here to test at the moment is that in the backend a user can specify the IFRC URL for a country and that this URL shows up correctly on the frontend?

If a URL has not been specified this "button" image will not show on that country's GO page.

Anything else to test?

If that's it, then this is tested and works.

GergiH commented 4 years ago

@teklal As far as I understood Natalie's comments right, I believe this is it. Thanks!

nanometrenat commented 4 years ago

Closing this ticket as it is now complete and in Production. It's the first ticket I ever logged in GO Github!

IM manual work required to paste in the IFRC URLs country by country, that is on my list for coming days.

In the meanwhile I have already done New Zealand :) as that is how I first noticed this. Woop!

nanometrenat commented 4 years ago

Note to self - see also "Verbose URLS" ticket https://github.com/IFRCGo/go-api/issues/357