redmine_github_hook
redmine_github_hook copied to clipboard
Fixes #64 - Authenticate request with WS repository management API key
This addresses #64 partially, as the API is mandatory but not optional as @fnkr mentioned in the issue.
@lacek Making it mandatory would be event better. I updated my issue. If you add "Fixes #64" to the description of your PR, it will close it automatically, as soon as this is merged.
Cool, thanks for your contribution - and thanks to @fnkr for bringing it up. I like this change, but I have a few comments and questions.
@koppen By Redmine's default Setting.sys_api_enabled is false. Thus this update breaks the plugin for user running with default settings, until they enable the sys_api_enabled and generate an API key.
I am not sure if this is the expected behavior. If not I would suggest creating separate configurations for this plugin.
I am a newbie to Ruby and Redmine Plugin development. Do you have any hint to run the test cases? I got the following error when trying to run them under Redmine 3.2.0 with Ruby 2.1.0:
redmine@23316d7deffc:~/redmine$ bundle exec rake redmine:plugins:test:functionals NAME=redmine_github_hook
/usr/bin/ruby2.1 -I"lib:test" -I"/home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/rake-10.5.0/lib" "/home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/rake-10.5.0/lib/rake/rake_test_loader.rb" "plugins/redmine_github_hook/test/functional/**/*_test.rb"
MiniTest::Unit::TestCase is now Minitest::Test. From /usr/lib/ruby/2.1.0/test/unit/testcase.rb:8:in `<module:Unit>'
/usr/lib/ruby/2.1.0/test/unit.rb:676:in `<class:Runner>': undefined method `_run_suite' for class `Test::Unit::Runner' (NameError)
from /usr/lib/ruby/2.1.0/test/unit.rb:261:in `<module:Unit>'
from /usr/lib/ruby/2.1.0/test/unit.rb:15:in `<module:Test>'
from /usr/lib/ruby/2.1.0/test/unit.rb:7:in `<top (required)>'
from /home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/activesupport-4.2.5/lib/active_support/dependencies.rb:274:in `require'
from /home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/activesupport-4.2.5/lib/active_support/dependencies.rb:274:in `block in require'
from /home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/activesupport-4.2.5/lib/active_support/dependencies.rb:240:in `load_dependency'
from /home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/activesupport-4.2.5/lib/active_support/dependencies.rb:274:in `require'
from /home/redmine/redmine/plugins/redmine_github_hook/test/functional/github_hook_controller_test.rb:3:in `<top (required)>'
from /home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:10:in `require'
from /home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:10:in `block (2 levels) in <main>'
from /home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:9:in `each'
from /home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:9:in `block in <main>'
from /home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:4:in `select'
from /home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:4:in `<main>'
rake aborted!
Command failed with status (1): [ruby -I"lib:test" -I"/home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/rake-10.5.0/lib" "/home/redmine/redmine/vendor/bundle/ruby/2.1.0/gems/rake-10.5.0/lib/rake/rake_test_loader.rb" "plugins/redmine_github_hook/test/functional/**/*_test.rb" ]
Do I need to rollback to older version of Redmine and Ruby for testing?
By Redmine's default Setting.sys_api_enabled is false. Thus this update breaks the plugin for user running with default settings, until they enable the sys_api_enabled and generate an API key.
I am not sure if this is the expected behavior. If not I would suggest creating separate configurations for this plugin.
I would very much like to not suddenly break the integration for everybody if it could be avoided. But I guess we could release a major version for this, and document it thoroughly.
Would it make sense to simply check for the existence of the API key and only verify it if it is present?
Do I need to rollback to older version of Redmine and Ruby for testing?
I hope not, but I admit that it has been a while since a last did any real development on the plugin, but I'm pretty sure it has never been particularly easy to run the test suite. Let me look into and I'll get back to you.
Alright, looks like the tests were still expecting Test::Unit. I've updated master with a switch to MiniTest, so you should be good to go now.
And yes, rake redmine:plugins:test is what I use to run my tests as well.
Would it make sense to simply check for the existence of the API key and only verify it if it is present?
Yes, this do provide flexibility for those who do not want API authentication. I've made the change, fixed and added test cases. Thanks for your fix, all tests pass now.
edit: I've messed file permission up after running test cases, rolled back and re-pushed