[WIP] Removing rails errors one by one.

[Souravirus]

Re: https://github.com/publiclab/plots2/pull/1832

Make sure these boxes are checked before your pull request is ready to be reviewed and merged. Thanks!

• [ ] all tests pass -- rake test:all
• [x] code is in uniquely-named feature branch, and has been rebased on top of latest master (especially if you've been asked to make additional changes)
• [x] pull request is descriptively named with #number reference back to original issue

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/wiki/contributing-to-public-lab-software

We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays. Please alert developers on plots-dev@googlegroups.com when your request is ready or if you need assistance.

Thanks!

[grvsachdeva]

I was going through the test report and it seems many errors are resolved. Awesome work @Souravirus !!!

[Souravirus]

Yeah I have now completely resolved the errors related to deliver_now methods. Now working on other errors. Are you seeing through Souravirus:rails-4.2_errors branch @Gauravano ?

[jywarren]

Reopened to trigger a build!!!

[Souravirus]

Hey @jywarren @ryzokuken Actually I was trying to correct a error: Error: Minitest::Result#test_first-timer_moderatednote(status=4)_hidden_to_normal_users_on_research_note_feed: ArgumentError: wrong number of arguments (given 3, expected 1) test/functional/notes_controller_test.rb:239:in `block in '

I cannot find how to correct this error. It is a very important error as many similar errors are there also which can be corrected if we are able to correct this error. I somehow find a solution, but I cannot verify myself that if it is the correct answer to it. So, the code snippet for the correction is: assert_select "div:match('class', ?)", "note-nid-#{node.id}", false

[ryzokuken]

I don't think assert_select takes three arguments.

[Souravirus]

But its working when I replace false with true, although test doesn't pass and what would you suggest to use instead of assert_select ".note-nid-#{node.id}", false

[ryzokuken]

@Souravirus take a look at https://apidock.com/rails/ActionController/Assertions/SelectorAssertions/assert_select

[Souravirus]

@ryzokuken It worked in older versions than 4.2 as told by insane-dreamer on that page. But in rails 4.2 escape interpolation inside assert_select is not working and showing this error: ArgumentError: wrong number of arguments (given 3, expected 1) So, that's the problem.

[ryzokuken]

@Souravirus Woah, 156 commits? Is something wrong?

[Souravirus]

No @ryzokuken , the thing is that I wanted to test with the latest commits and this jywarren:rails-4.2 branch was way behind than the master branch. So I just merged the latest commits in the publiclabs main branch to the current branch.

[Souravirus]

Hey @jywarren please update this rails-4.2_errors branch. So that I can test my PR's with latest commits.

[jywarren]

Hm, perhaps it's best to do a git rebase, to ensure the changes are added properly, rather than merge the latest commits in here. But if it worked, it worked! What would you like me to do here? Thanks for tackling this!!

[jywarren]

Aha - did you see the changes in https://github.com/publiclab/plots2/pull/2472 -- this will get your builds running again!

Try backing up this branch -- don't lose anything! and then doing a git rebase over the lastest master -- https://www.atlassian.com/git/tutorials/merging-vs-rebasing

It'll take some care to do right, but it's worth it!

[Souravirus]

@jywarren what I wanted is that to update the jywarren:rails-4.2 branch to publiclab:master branch. So, that I don't have to send a PR containing all the latest PR's and the files changed becomes easy to read. I will take care of rebasing the PR's.

[jywarren]

Ah, i see! OK, can you try opening a new PR against jywarren:rails-4.2 that rebases it over the latest publiclab:master, and then I can accept that pull request? Then you'll have to make this PR rebase over the result. I agree that makes sense as we'll be able to see the changes of this apart from the rest of the work.

OR you could rebase this directly over publiclab:master and open a separate PR in parallel to jywarren:rails-4.2 but you'd then want to clearly link the two together in your comments, so we don't get confused. Make sense? Thank you!!!

[Souravirus]

Okk I understood what you are telling. I will soon open a PR on the latest commits. Thanks.