Defect #32935

Rails 6: fix set role_ids

Added by Pavel Rosický over 1 year ago. Updated 7 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Rails support
Target version:-
Resolution:Fixed Affected version:

Description

fixes two failures

Failure:
Redmine::ApiTest::MembershipsTest#test_PUT_/memberships/:id.xml_with_invalid_parameters_should_return_errors [/redmine/test/integration/api_test/memberships_test.rb:152]:
Expected response to be a <422: unprocessable_entity>, but was a <204: No Content>
Response body: .
Expected: 422
  Actual: 204

Failure:
GroupTest#test_user_roles_should_updated_when_updating_group_roles [/redmine/test/unit/group_test.rb:113]:
Expected: [2]
  Actual: [4]
rails test test/unit/group_test.rb:101

member.rb.patch Magnifier (824 Bytes) Pavel Rosický, 2020-02-03 00:42


Related issues

Related to Redmine - Feature #29914: Migrate to Rails 6.1 New

Associated revisions

Revision 20899
Added by Go MAEDA 7 months ago

Rails 6.1: fix set roleids (#29914, #32935).

Patch by Pavel Rosický.

History

#1 Updated by Marius BALTEANU over 1 year ago

#2 Updated by Go MAEDA over 1 year ago

  • Category set to Rails support

#3 Updated by Pavel Rosický 7 months ago

@marius - I see you've finally started working on this. Just to let you know, there's a mistake in this patch. The removed part simply shouldn't be removed, the rest is ok. I'll send you a few other patches because there was a new major release of Rails and Ruby since the last time :)

#4 Updated by Marius BALTEANU 7 months ago

Pavel Rosický wrote:

@marius - I see you've finally started working on this. Just to let you know, there's a mistake in this patch. The removed part simply shouldn't be removed, the rest is ok. I'll send you a few other patches because there was a new major release of Rails and Ruby since the last time :)

Indeed, I've started work on preparing patches to update Rails to latest version (6.1.3.1) and you can see here the results. I've only 3 failing tests, 2 are caused by this patch and one I need to investigate because it passes on my local environment.

Anyway, please take a look on all the commits that I've pushed and feel free to post new patches or improved solutions for the existing patches.

My plan is the following:
- first priority: make all the tests pass (I will post the patches on #29914)
- refactor some existing solutions (on separate issues)
- adopt some new Rails features (like Zeitwerk) (on separate issues)
- inform Jean-Philippe that we have the patches ready for review and hopefully, we get an approval from him to commit them as soon as possible.

Pavel, thank you for all of your work on this, your patches were really useful.

#5 Updated by Pavel Rosický 7 months ago

you're welcome, the last failure is related to #32936

the class attribute isn't inherited to subclasses, that's why your change doesn't work
https://gitlab.com/redmine-org/redmine/-/commit/f0afce30b79c8d54af9d082e4b3883ef8d672c44

after fixing it tests should be green.

#6 Updated by Marius BALTEANU 7 months ago

Pavel Rosický wrote:

you're welcome, the last failure is related to https://www.redmine.org/issues/32936

the class attribute isn't inherited to subclasses, that's why your change doesn't work
https://gitlab.com/redmine-org/redmine/-/commit/f0afce30b79c8d54af9d082e4b3883ef8d672c44

after fixing it tests should be green.

Sorry, I was blind and I didn't see that I've disabled the test adapter on the ActiveJob and not on the ActionMailer. Thanks for pointing this out.

#7 Updated by Marius BALTEANU 7 months ago

  • Tracker changed from Patch to Defect

#8 Updated by Go MAEDA 7 months ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch as a part of #29914.

Also available in: Atom PDF