Project

General

Profile

Actions

Defect #32935

closed

Rails 6: fix set role_ids

Added by Pavel Rosický about 4 years ago. Updated about 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Rails support
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:
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

Files

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

Related issues

Related to Redmine - Feature #29914: Migrate to Rails 6.1 with Zeitwerk autoloadingClosedGo MAEDA

Actions
Actions #1

Updated by Marius BĂLTEANU about 4 years ago

  • Related to Feature #29914: Migrate to Rails 6.1 with Zeitwerk autoloading added
Actions #2

Updated by Go MAEDA about 4 years ago

  • Category set to Rails support
Actions #3

Updated by Pavel Rosický about 3 years ago

Marius Ionescu - 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 :)

Actions #4

Updated by Marius BĂLTEANU about 3 years ago

Pavel Rosický wrote:

Marius Ionescu - 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.

Actions #5

Updated by Pavel Rosický about 3 years 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.

Actions #6

Updated by Marius BĂLTEANU about 3 years 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.

Actions #7

Updated by Marius BĂLTEANU about 3 years ago

  • Tracker changed from Patch to Defect
Actions #8

Updated by Go MAEDA about 3 years ago

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

Committed the patch as a part of #29914.

Actions

Also available in: Atom PDF