search

schneidmaster / gitreports.com

Git Reports is a free service that lets you set up a stable URL for anonymous users to submit bugs and other Issues to your GitHub repositories.
MIT License
134 stars 26 forks source link

Issue #74 follow up messages #112

Closed hd719 closed 6 years ago

hd719 commented 7 years ago

Hey @schneidmaster ! For issue #74.

So even w/ StackOver Flow I could not find out why the captcha was blacked out, but I am hoping what I have added to the app/views/repositories/submitted.erb does work.

If the code does not work, please do let me know and I will try to fix it.

-Thank you.

schneidmaster commented 7 years ago

Looks pretty good, a couple of comments:

hd719 commented 7 years ago

@schneidmaster Cool!

Just a quick question, what do you mean by this 

I think we should have this disabled by default and enabled via a configuration option. Currently, all of the copy on the site is either user-configurable or draws from the locales -- a few users have submitted translations. Obviously I don't know how to translate the new copy from English, but for their setups, I don't want to start displaying some non-translated English by default.

-Thank you.

Also I will revert the schema to how it was before. Lastly how would I add tests, I am particularly new in that area.

schneidmaster commented 7 years ago

I think that by default, we should not show the issue link, and add a configuration option to the Repository model to allow users to turn on the issue link via a checkbox. Locales are something used by Rails to support translations of text in the application -- currently, all of the text in the app either uses locales (with some users using French and Polish) or is user-configurable (i.e. they type what they want in a form). So if we enable the issue link by default, users that are expecting the site to currently all be in French or Polish would suddenly see some English showing up when issues are submitted. Making it opt-in helps with that since we don't break current issue pages.

You should look in the spec/features folder for examples of existing feature tests.

hd719 commented 7 years ago

Oh okay, I get what you are saying. So do you want me to take out the issue link?

But in the meantime I will revert the schema and write a test for this feature.

hd719 commented 7 years ago

Hey @schneidmaster so I added the test for the feature, but when I tried to test to see if it would work, but I got this error. It said it could not load spec_helper_file so I tried rspec init after and that resulted in an error as well. 

Hamels-MacBook-Air:features HD$ rspec spec repositories_feature_spec.rb
/Users/HD/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.3.1/lib/rspec/core/configuration.rb:1280:in
 `require': cannot load such file -- spec_helper (LoadError)
        from /Users/HD/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.3.1/lib/rspec/core/configurati
on.rb:1280:in `block in requires='
        from /Users/HD/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.3.1/lib/rspec/core/configurati
on.rb:1280:in `each'
        from /Users/HD/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.3.1/lib/rspec/core/configurati
on.rb:1280:in `requires='
        from /Users/HD/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.3.1/lib/rspec/core/configurati
Hamels-MacBook-Air:features HD$ ls
authentications_feature_spec.rb repositories_feature_spec.rb
Hamels-MacBook-Air:features HD$ rspec init
/Users/HD/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.3.1/lib/rspec/core/configuration.rb:1280:in
 `require': cannot load such file -- spec_helper (LoadError)
        from /Users/HD/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.3.1/lib/rspec/core/configurati
on.rb:1280:in `block in requires='

Also I think I reverted the schema back to how it was I did a rake db:rollback and hopefully that worked.

-Let me know if I need to fix anything I will do it ASAP.

-Thank you.

hd719 commented 7 years ago

@schneidmaster Hey any update on this?

schneidmaster commented 7 years ago

@hd719 your test command is incorrect -- you need to be in the project root and run rspec spec/features/repositories_feature_spec.rb

hd719 commented 7 years ago

@schneidmaster Hey so I am having trouble with writing out the tests, but this is what I have so far.

-Thank you.

schneidmaster commented 7 years ago

You can't do expect_to visit ... -- visit is a command that tells Capybara to visit the provided URL. If you're trying to test that the page is on a particular URL you're looking for expect(page).to have_current_path(whatever_path)

hd719 commented 7 years ago

Hey @schneidmaster I added the new path that test's for the here button that should link to the owners repository issue, but for some reason it is not passing.

hd719 commented 7 years ago

Hey @schneidmaster sorry for the bother, but any update with this I would really like to get this PR merged soon.

I am currently in a coding bootcamp and we have to contribute to a set number of repos. and this one is currently on my list.

P.S. By the way thank you all the help very much appreciated.

schneidmaster commented 7 years ago

Hey @hd719. Looks like this project is actually on an older version of Capybara, if you upgrade Capybara to >= 2.5 that syntax should work.

As I said above, we'll also need to add a configuration option to the repository itself, to make this link opt-in for non-English users.

hd719 commented 7 years ago

@schneidmaster Alright sounds good! I am going to try and add a configuration option so the link would work for non-English users.

Real quick in order for me to do this, where should I start? Any advice would be appreciated.

schneidmaster commented 7 years ago

allow_issue_title is a pretty good example -- it is a configuration option that allows users to set the issue title in GitHub, while your configuration option will be to show this link when the issue is submitted. Look at the files from this search to get an idea of where your option needs to be added.

schneidmaster commented 6 years ago

Closing this PR for now since it seems to have gone stale. Feel free to open a new PR if you decide to take this feature back up.