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

Verify controller keys within `register_module` #1495

Merged
merged 1 commit into from Oct 25, 2018

Conversation

Projects
None yet
3 participants
@westonganger
Copy link
Contributor

commented Oct 23, 2018

Verify controller keys within register_module

Closes #1490

'action' => 'index'
}
}

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Oct 23, 2018

Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end.

'action' => 'index'
}
}

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Oct 23, 2018

Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end.

end

defined_controllers.each do |controller_val|
if controller_val.present?

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Oct 23, 2018

Style/Next: Use next to skip iteration.

it "registers a module definition into global list of modules" do
::RegisterModuleDummyController = true ### mock the existence of the constant

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Oct 23, 2018

Naming/ConstantName: Use SCREAMING_SNAKE_CASE for constants.

'controller' => 'bad_module',
'action' => 'index'
}]
},

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Oct 23, 2018

Style/TrailingCommaInHashLiteral: Avoid comma after the last item of a hash.

@@ -74,5 +87,19 @@ def action_matches?(navi, params)
def remove_slash(str)
str.gsub(/^\//, '')
end

def validate_controllers_existence(controllers)

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Oct 24, 2018

Lint/UnusedMethodArgument: Unused method argument - controllers. If it's necessary, use _ or _controllers as an argument name to indicate that it won't be used. You can also write as validate_controllers_existence(*) if you want the method to accept any arguments but don't care about them.

@@ -74,5 +87,19 @@ def action_matches?(navi, params)
def remove_slash(str)
str.gsub(/^\//, '')
end

def self.validate_controllers_existence(controllers)

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Oct 24, 2018

Lint/IneffectiveAccessModifier: private (on line 64) does not make singleton methods private. Use private_class_method or private inside a class << self block instead.

This comment has been minimized.

Copy link
@tvdeyen

tvdeyen Oct 25, 2018

Member

I would actually prefer to move the method into a

class << self
end

block and move the two existing class methods into that as well.

@tvdeyen
Copy link
Member

left a comment

Thank you. Great contribution. I have some small things that would be great if they get addressed.

@@ -74,5 +87,19 @@ def action_matches?(navi, params)
def remove_slash(str)
str.gsub(/^\//, '')
end

def self.validate_controllers_existence(controllers)

This comment has been minimized.

Copy link
@tvdeyen

tvdeyen Oct 25, 2018

Member

I would actually prefer to move the method into a

class << self
end

block and move the two existing class methods into that as well.


begin
controller_name.constantize
rescue RuntimeError

This comment has been minimized.

Copy link
@tvdeyen

tvdeyen Oct 25, 2018

Member

Could we rescue NameError here instead? This is what Rails raises if the constant is undefined.

Suggested change
rescue RuntimeError
rescue NameError
@westonganger

This comment has been minimized.

Copy link
Contributor Author

commented Oct 25, 2018

Done.

@tvdeyen
Copy link
Member

left a comment

Thank you 🎉

@tvdeyen tvdeyen merged commit 5114d5a into AlchemyCMS:master Oct 25, 2018

3 checks passed

Hound No violations found. Woof!
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tvdeyen tvdeyen added this to the 4.2 milestone Jan 26, 2019

tvdeyen added a commit to tvdeyen/alchemy_cms that referenced this pull request Mar 29, 2019

v4.2.0.rc1
- Use element name local in generators [AlchemyCMS#1556](AlchemyCMS#1556) ([tvdeyen](https://github.com/tvdeyen))
- Remove invalid bytecode handler [AlchemyCMS#1555](AlchemyCMS#1555) ([tvdeyen](https://github.com/tvdeyen))
- Separate render element calls [AlchemyCMS#1554](AlchemyCMS#1554) ([tvdeyen](https://github.com/tvdeyen))
- Expose the element into partials as local object [AlchemyCMS#1553](AlchemyCMS#1553) ([tvdeyen](https://github.com/tvdeyen))
- Allow admins to switch all languages [AlchemyCMS#1552](AlchemyCMS#1552) ([tvdeyen](https://github.com/tvdeyen))
- Raise targeted Ruby version to 2.3 [AlchemyCMS#1545](AlchemyCMS#1545) ([tvdeyen](https://github.com/tvdeyen))
- Introduces an Elements finder class [AlchemyCMS#1544](AlchemyCMS#1544) ([tvdeyen](https://github.com/tvdeyen))
- Fixate sqlite dep for bug fix [AlchemyCMS#1543](AlchemyCMS#1543) ([tvdeyen](https://github.com/tvdeyen))
- Upgrade shoulda-matchers to version 4.0.0 [AlchemyCMS#1542](AlchemyCMS#1542) ([depfu](https://github.com/apps/depfu))
- Upgrade factory_bot_rails to version 5.0.1 [AlchemyCMS#1540](AlchemyCMS#1540) ([depfu](https://github.com/apps/depfu))
- Use Flatpickr as Datepicker [AlchemyCMS#1533](AlchemyCMS#1533) ([mamhoff](https://github.com/mamhoff))
-  Use system tests over feature specs [AlchemyCMS#1528](AlchemyCMS#1528) ([tvdeyen](https://github.com/tvdeyen))
- Flexible width for admin navigation entry labels [AlchemyCMS#1527](AlchemyCMS#1527) ([tvdeyen](https://github.com/tvdeyen))
- Responsive elements window and sidebar [AlchemyCMS#1519](AlchemyCMS#1519) ([tvdeyen](https://github.com/tvdeyen))
- Change element eye icon on public state [AlchemyCMS#1517](AlchemyCMS#1517) ([oniram88](https://github.com/oniram88))
- Maximize element window if Tinymce is fullscreen [AlchemyCMS#1515](AlchemyCMS#1515) ([tvdeyen](https://github.com/tvdeyen))
- Remove cells in favour of fixed elements [AlchemyCMS#1514](AlchemyCMS#1514) ([tvdeyen](https://github.com/tvdeyen))
- Feature: Autogenerate nestable elements [AlchemyCMS#1513](AlchemyCMS#1513) ([tvdeyen](https://github.com/tvdeyen))
- Allow "data" key for module navigations [AlchemyCMS#1512](AlchemyCMS#1512) ([mamhoff](https://github.com/mamhoff))
- Allow to define layout for page previews [AlchemyCMS#1500](AlchemyCMS#1500) ([westonganger](https://github.com/westonganger))
- Fix capitalization for login/logout/leave [AlchemyCMS#1497](AlchemyCMS#1497) ([westonganger](https://github.com/westonganger))
- Verify controller keys within `register_module` [AlchemyCMS#1495](AlchemyCMS#1495) ([westonganger](https://github.com/westonganger))
- Update bundled Tinymce to 4.8.3 [AlchemyCMS#1491](AlchemyCMS#1491) ([tvdeyen](https://github.com/tvdeyen))
- Use dynamic attributes in factories [AlchemyCMS#1484](AlchemyCMS#1484) ([tvdeyen](https://github.com/tvdeyen))
- Migrating to active_model_serializers ~> 0.10.0 [AlchemyCMS#1478](AlchemyCMS#1478) ([pmashchak](https://github.com/pmashchak))
- Replace picture galleries with nestable elements [AlchemyCMS#1358](AlchemyCMS#1358) ([tvdeyen](https://github.com/tvdeyen))
- Add a compact nested element style [AlchemyCMS#1357](AlchemyCMS#1357) by [tvdeyen](https://github.com/tvdeyen)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.