Patch #33337

Clean-up workflows controller

Added by Vincent Robert 12 months ago. Updated 4 days ago.

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

0%

Category:Code cleanup/refactoring
Target version:5.0.0

Description

Hello

Here is a patch which aims to refactor the WorkflowsController.
The patch does not change the behavior of the application, but use smaller methods and clearer code. The routes are also updated.

My goal was to use a specific action for each route, and do not use a same method for different purposes (so avoid checking if the request-method is "post" inside actions).

Hope this may help.

cleanup_workflows_controller.diff Magnifier (17.1 KB) Vincent Robert, 2020-04-21 21:10

0002-Fix-route-name.patch Magnifier (868 Bytes) Marius BALTEANU, 2021-04-04 13:14

0001-Clean-up-workflows-controller.patch Magnifier (17.4 KB) Marius BALTEANU, 2021-04-04 13:14

Associated revisions

Revision 20941
Added by Go MAEDA 4 days ago

Clean-up workflows controller (#33337).

Patch by Vincent Robert and Marius BALTEANU.

History

#1 Updated by Marius BALTEANU about 1 month ago

  • Assignee set to Marius BALTEANU

#2 Updated by Marius BALTEANU about 1 month ago

  • File 0001-Clean-up-workflows-controller.patch added
  • File 0002-Fix-route-name.patch added
  • Target version set to Candidate for next major release

Thanks Vincent for the patch, I tried it yesterday and it looks great from my point of view.

I'm attaching a rebased version of the initial patch and a second patch that fixes to route name (workflows_permissions_path should be permissions_workflows_path). All the tests pass (https://gitlab.com/redmine-org/redmine/-/commit/4ef4ea5047323756a3241eea660e11a383ea7711) with both patches.

I'm going to review the tests in the following days.

#3 Updated by Marius BALTEANU 13 days ago

  • File deleted (0001-Clean-up-workflows-controller.patch)

#4 Updated by Marius BALTEANU 13 days ago

  • File deleted (0002-Fix-route-name.patch)

#5 Updated by Marius BALTEANU 13 days ago

Update the patches for current trunk, the tests still pass.

Setting target to 5.0.0, I think it's a nice refactoring.

#6 Updated by Go MAEDA 4 days ago

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

Committed the patch. Thank you for your contribution.

#7 Updated by Go MAEDA 4 days ago

  • Subject changed from Clean-up Workflows Controller to Clean-up workflows controller

#8 Updated by Vincent Robert 4 days ago

Thank you for updating and committing this patch.

Also available in: Atom PDF