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

Fix backward compatible Pundit include #2383

Merged

Conversation

fbuys
Copy link
Contributor

@fbuys fbuys commented May 23, 2023

This commit fixes #2378 By adding a conditional that includes Pundit::Authorization when it is defined else it includes Pundit.

This change ensures that we remain compatible with Pundit < 2.2.0

To test with Pundit "~> 2.1.0", we run:

 bundle exec appraisal pundit21 rspec spec/controllers/concerns/administrate/punditize_spec.rb   

See: #2141
See: #1068

@fbuys fbuys force-pushed the fbuys/2378-fix-uninitialized-constant branch 2 times, most recently from 3575d34 to 846cd19 Compare May 23, 2023 23:56
Copy link
Contributor Author

@fbuys fbuys left a comment

Choose a reason for hiding this comment

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

Please let me know if I need to update any docs.

if Pundit.const_defined?(:Authorization)
include Pundit::Authorization
else
include Pundit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fbuys fbuys force-pushed the fbuys/2378-fix-uninitialized-constant branch from 846cd19 to 8728917 Compare May 24, 2023 22:05
Copy link
Collaborator

@pablobm pablobm left a comment

Choose a reason for hiding this comment

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

Thank you!

@pablobm
Copy link
Collaborator

pablobm commented May 26, 2023

We appear to be having CI issues 😓 I'll merge after we solve them.

@pablobm
Copy link
Collaborator

pablobm commented May 26, 2023

CI should be fixed in master now. Can you try rebase, please?

This commit fixes: thoughtbot#2378
By adding a conditional that includes Pundit::Authorization when it is
defined else it includes Pundit.

This change ensures that we remain compatible with Pundit < 2.2.0

To test with Pundit "~> 2.1.0", we run:
```
bundle exec appraisal pundit21 rspec spec/controllers/concerns/administrate/punditize_spec.rb
```

See: thoughtbot#2141
See: thoughtbot#1068
@fbuys fbuys force-pushed the fbuys/2378-fix-uninitialized-constant branch from 8728917 to 1a4147b Compare May 26, 2023 22:50
@fbuys
Copy link
Contributor Author

fbuys commented May 26, 2023

CI should be fixed in master now. Can you try rebase, please?

No problem, I have rebased and pushed again ✅

@pablobm pablobm merged commit 403d688 into thoughtbot:main May 27, 2023
@pablobm
Copy link
Collaborator

pablobm commented May 27, 2023

Great stuff! Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrading to 0.18.0 results in uninitialized constant Pundit::Authorization
2 participants