RefugeRestrooms / refugerestrooms

REFUGE restrooms indexes and maps safe restroom locations for trans, intersex, and gender nonconforming individuals.
http://www.refugerestrooms.org
GNU Affero General Public License v3.0
891 stars 263 forks source link

Add a locale switcher #620

Closed DeeDeeG closed 4 years ago

DeeDeeG commented 4 years ago

Context

Summary of Changes

Checklist

Screenshots

Before

After

DeeDeeG commented 4 years ago

If anyone knows how to make this test passing again, I'd appreciate it a lot! :heart:

Failures:

  1) the contact process should show a generic contact when contact is not from restroom form
     Failure/Error: visit restroom_path restroom

     ActionController::UrlGenerationError:
       No route matches {:action=>"show", :controller=>"restrooms", :locale=>#<Restroom id: 232, name: "Mission Creek Cafe", street: "1800 Market St", city: "San Francisco", state: "CA", accessible: false, unisex: false, directions: "Direction", comment: "Comment", latitude: nil, longitude: nil, created_at: "2020-04-11 00:04:34", updated_at: "2020-04-11 00:04:34", downvote: 11, upvote: 22, country: "US", changing_table: false, edit_id: 232, approved: true>}, missing required keys: [:id]
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/journey/formatter.rb:57:in `generate'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:744:in `generate'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:775:in `generate'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:822:in `url_for'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:273:in `call'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:214:in `call'
     # /usr/local/bundle/gems/actionpack-5.2.4.2/lib/action_dispatch/routing/route_set.rb:331:in `block (2 levels) in define_url_helper'
     # ./spec/features/contacts_spec.rb:7:in `block (2 levels) in <top (required)>'

Finished in 40.73 seconds (files took 1.95 seconds to load)
62 examples, 1 failure

Failed examples:

rspec ./spec/features/contacts_spec.rb:4 # the contact process should show a generic contact when contact is not from restroom form

It seems like when visit restroom_path restroom is invoked here, a garbled :locale value gets piped in that has way too much unrelated data in it, and so it isn't valid as a locale. And the app gets rather unhappy about it.


Viewing restroom listings, and using the contacts page both work locally in Docker.

I think the test itself is broken here, and the app isn't misbehaving (other than not matching up well with the test anymore).

Since this test is related to using the "contact" page from a restroom listing, and that functionality was removed in https://github.com/RefugeRestrooms/refugerestrooms/pull/487, I'm somewhat tempted to just disable or greatly simplify this test.

mi-wood commented 4 years ago

That test still sort of acts as a regression tests and tests navigation to Contact.

Right now, it's implicitly filling in the parameters of restroom_path and setting :locale to the restroom. You can explicitly set it as restroom_path(:id => restroom.id).

https://stackoverflow.com/questions/31224170/no-route-matches-action-show

DeeDeeG commented 4 years ago

Thanks, @mi-wood, tests are passing now!

I think this is ready to merge.

DeeDeeG commented 4 years ago

Gong to put these changes on staging for a bit, but it seems worth another deploy if they behave well.

DeeDeeG commented 4 years ago

Turns out this does break some of the locale-prefix-less routes, like /about, /business_info, /contact, /signs and the hidden /text. It doesn't break /restrooms/new, /restrooms/[id], or /api/docs though.

locale-prefixed routes are doing fine, such as /en/signs, /fr/contact, /pt-BR/business_info etc.

Either I need to figure out a fix (or someone can explain one to me/commit it themselves) or perhaps revert this PR for now in develop branch.

I find routing in Rails kind of tricky.

DeeDeeG commented 4 years ago

Reverted for now, as this wasn't working as intended.

This was working with just the ?locale=[your-locale-here] URL parameters, but was still kinda broken with the /[your-locale-here]/ URL prefixes.

I would really like to get the URL prefixes working, but I need to read up more on the Rails router. (If anyone knows how to do this, feel free to step in!)

Also, there are hard-coded HTML anchor (<a>) tags from about to business_info, and vice-versa, but these are inside Internationalization (I18n) strings. I have to look up the interaction between haml and I18n here to properly translate the dynamic link with link_to and [page-here]_path helpers.