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

Interference Marketable Routes & ActiveStorage fixed #3385

Conversation

Obi-TOB
Copy link
Contributor

@Obi-TOB Obi-TOB commented Jul 26, 2018

Closes #3383

@bricesanchez
Copy link
Member

Thank you very much @Obi-TOB !

Should we also add rails/active_storageslug in friendly_id_reserved_words ?

We don't want to create a page with this slug and not able to reach it.

@Obi-TOB
Copy link
Contributor Author

Obi-TOB commented Jul 27, 2018

Hi @bricesanchez,

I added all words from all reserved_paths to the friendly_id_reserved_words.

I guess this might go a bit far... as this probably means no friendly_id slugs with rails or active storage. I'm not entirely sure how friendly id stores the slugs, but I think we need to exclude each word on it's own.

Let me know what you think...

self.friendly_id_reserved_words = %w(
index new session login logout users refinery admin images
)
) + self.reserved_paths.map { |path| path.split('/') }.flatten.uniq - [""]
Copy link
Member

Choose a reason for hiding this comment

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

You probably just need to add rails because we already just add refinery or admin in friendly_id_reserved_words even if there are sub path to these paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I now add only the first elements from each element of reserved_paths to friendly_id_reserved_words, so in the case we have only one element '/rails/active_storage' it will only take 'rails'. If you add several paths it will remove all 'root'-fragments of the paths

Copy link
Member

@parndt parndt Jul 29, 2018

Choose a reason for hiding this comment

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

maybe we could use:

self.friendly_id_reserved_words = %w(
  index new session login logout users refinery admin images
) | reserved_paths.map { |path| path.split('/') }.flatten.uniq.reject(&:blank?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to

self.friendly_id_reserved_words = %w(
  index new session login logout users refinery admin images
) | self.reserved_paths.map { |path| path.split('/').reject(&:blank?).first}.flatten.uniq

to take only the all root_fragments of the reserved paths.

@bricesanchez
Copy link
Member

Thanks @Obi-TOB !

I've just seen that the new config reserved_paths is missing pages initializer template : https://github.com/refinery/refinerycms/blob/master/pages/lib/generators/refinery/pages/templates/config/initializers/refinery/pages.rb.erb

Could you add it ?

@bricesanchez
Copy link
Member

Closed in favor of #3387.

@bricesanchez bricesanchez mentioned this pull request Aug 21, 2018
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants