Skip to content
This repository has been archived by the owner on Aug 10, 2021. It is now read-only.

Better handle locale in request.path - solve #268 #269

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kulgar
Copy link

@Kulgar Kulgar commented Oct 15, 2019

I modified the code a bit to better handle locale (fr, en) in request path, see #268.

I also removed the '/' that was automatically added at the beginning of a slug for two reasons:

  • First, I don't see why is that necessary, I think this was because of the use of "request.path" which always starts with a '/'. As my pull request doesn't user request.path anymore, adding the '/' isn't necessary anymore - I think.
  • Second, it adds the '/' only for the default locale. Slug is a translatable field, and the '/' isn't automatically added for other languages, so that is a little bit weird.

To keep a backward compatibility, I added [path, "/#{path]"] in Spree::Page find methods.
I updated rspecs accordingly and they are all passing.

I wondered if I should also add a unit test for locale in path? You tell me.

Another question: why the gem isn't using friendly_id just like spree_core for products? Any particular reasons?

Handled locale in request.path, used path_parameters[:path] instead
Updated rspecs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant