-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
Fix Alchemy.user_class_name constant conflict #1724
Conversation
Thanks. I think that makes sense to avoid constant name conflicts. But could you please make the change in the |
Yeah, that makes more sense. I made getter/setter methods for the user_class_name variable and removed it from mattr_accessor. Just tell me if there's something you want to change. |
Maybe this might need a bit more testing.. I'm not sure if its related but now i'm seeing this when I'm trying to fold pages:
Removing Tell me if you want me to create a new pr for that fix also. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks this is very good. Please add a test as well.
Unfortunately the Github CI is having some issues. I will look into this soon. Meanwhile please run tests locally (they are very fast) and add a test for the namespace fix. Thanks again for your contribution |
For the changelog (that gets generated from PR titles) it would be better to rename this PR |
Ah, missed reading through the contribution notes. Is it possible to run Hound locally also? Added a test for getting with prefix Still have 2 failures. Unrelated? Also, what to do with
|
The two failing specs are unrelated. |
@mickenorlen would you please rebase with latest |
- Do not use deprecated methods [#1737](#1737) ([tvdeyen](https://github.com/tvdeyen)) - Order contents by their position in its element [#1733](#1733) ([tvdeyen](https://github.com/tvdeyen)) - Eager load relations in elements trash [#1732](#1732) ([tvdeyen](https://github.com/tvdeyen)) - Run CI builds with Sprockets 3.7.2 [#1731](#1731) ([tvdeyen](https://github.com/tvdeyen)) - Re-organize development dependencies [#1730](#1730) ([tvdeyen](https://github.com/tvdeyen)) - Update pr template [#1729](#1729) ([tvdeyen](https://github.com/tvdeyen)) - Generate views without _view in the filename [#1728](#1728) ([tvdeyen](https://github.com/tvdeyen)) - Fix CI Builds [#1727](#1727) ([tvdeyen](https://github.com/tvdeyen)) - Fix page tagging condition: should_attach_to_menu? [#1725](#1725) ([mickenorlen](https://github.com/mickenorlen)) - Fix Alchemy.user_class_name constant conflict [#1724](#1724) ([mickenorlen](https://github.com/mickenorlen))
Alchemy 4.4.2 has a fix for constant lookup issues (AlchemyCMS/alchemy_cms#1724) That prefixes the user class with `::` In order to be compatible with older Alchemy versions we use the class#name instead.
What is this pull request for?
After fresh installation in rails 6, after creating the index page, I got this error when trying to load page tree:
Then I found this which seems to be a working solution:
https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/18329
https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/18329/diffs
Notable change
Only change made is that I replaced all instances of:
Alchemy.user_class_name
with:
"::#{Alchemy.user_class_name}"
I just started to test alchemy out so I can't tell if this has some other consequences as of yet. Also I'm not sure the
stamper_class_name
should be replaced in the same manner.