rubycorns / rorganize.it

A web app to coordinate railsgirls project groups
https://rorganize.it
GNU Affero General Public License v3.0
41 stars 23 forks source link

PG/ rails upgrade throws error on our :through relationship in rails `5.1.5` #619

Closed lislis closed 4 years ago

lislis commented 6 years ago

When trying to run the newly upgraded setup in production, we get this error when we try to open the people page:

ActiveRecord::HasManyThroughOrderError - Cannot have a has_many :through association 'Group#students' which goes through 'Group#student_memberships' before the through association is defined.:
  app/controllers/groups_controller.rb:55:in `show'

This is new and has not happened before the upgrade. I am very clueless clueless

lislis commented 6 years ago

@til can I ping you about this? From the tutorials I've read I think this is (still) correct?

til commented 6 years ago

Hi @lislis! Dunno, I've never seen this error before. The error message sounds quite well explained on first look - did you try fixing it by reordering the definitions of the associations in Group?

Is that a problem currently on production, or did you try and then revert?

lislis commented 6 years ago

@til I thought it was the ordering at first, too. But that only made a couple of tests pass (from 71 to 59 failing tests). What may be note worthy is that the controller spec for the Memberships controller fails completely like here https://travis-ci.org/rubycorns/rorganize.it/jobs/361766421#L735 And I can't really explain that.

I reset git HEAD on production manually because we couldn't fix it the last time and it doesreak production.

I have a fix in place with a different rails version which I'm deploying in a few minutes.

This has_many_through error only appears with Rails version 5.1.5. In the update to 5.0.7 it seems to be fine. So my fix will update to that version for now.

til commented 6 years ago

I think many of the errors such as the ones in the memberships controller spec are caused by an API change of the test helpers. So e.g. it says

Failure/Error: delete :destroy, id: membership.id ArgumentError: unknown keyword: id

the line in the spec should be instead written as

delete :destroy, params: { id: membership.id }

I think this has changed some time ago, maybe we were still running on a deprecated behaviour, but in 5.1 it finally is too late. I couldn't find anything in the changelog, but here is the documentation for the current version of the request helpers:

http://api.rubyonrails.org/classes/ActionController/TestCase/Behavior.html#method-i-process

process is called by the actual helper methods put, delete, get etc. which we use.

lislis commented 6 years ago

That would make a lot of sense. I guess we need to update this at some point anyway :) Thanks for looking into it!

sareg0 commented 5 years ago

Do you know if this is still happening? It couldn't reproduce the error, but perhaps I need to ssh into the server?

lislis commented 5 years ago

it should already be happening locally with some data

vsmart commented 4 years ago

I think we can close this - we've run successfully on rails 5.2 for a while now.

lislis commented 4 years ago

nod