Top Menu

Jump to content
Home
    Modules
      • Projects
      • Activity
      • Work packages
      • Gantt charts
      • Calendars
      • Team planners
      • Boards
      • News
    • Getting started
    • Introduction video
      Welcome to OpenProject Community
      Get a quick overview of project management and team collaboration with OpenProject. You can restart this video from the help menu.
    • Help and support
    • Upgrade to Enterprise edition
    • User guides
    • Videos
    • Shortcuts
    • Community forum
    • Enterprise support
    • Additional resources
    • Data privacy and security policy
    • Digital accessibility (DE)
    • OpenProject website
    • Security alerts / Newsletter
    • OpenProject blog
    • Release notes
    • Report a bug
    • Development roadmap
    • Add and edit translations
    • API documentation
  • Sign in
      Forgot your password?

      or sign in with your existing account

      OpenProject ID Google

Side Menu

  • Overview
  • Activity
    Activity
  • Roadmap
  • Work packages
    Work packages
  • Gantt charts
    Gantt charts
  • Calendars
    Calendars
  • Team planners
    Team planners
  • Boards
    Boards
  • News
  • Forums

Content

Development
  1. OpenProject
  2. Forums
  3. Development
  4. Delete work package action doesn't use ::WorkPackages::DestroyService

Delete work package action doesn't use ::WorkPackages::DestroyService

Added by Dmitry Kazakov over 7 years ago

Hi, all!

I’m trying to implement a feature for OpenProject: automatically mark work packages if they have non-completed predecessors. To avoid recursive db-requests on every search, I added a boolean column to the work package db table and try to keep it in sync when any package created/updated/destroyed. I have modified ::WorkPackages::UpdateService and ::WorkPackages::DestroyService to do all the needed work.

But it seems like ::WorkPackages::DestroyService is never called, when a work package is deleted using the frontend, WorkPackages::BulkController is used instead.

Is there any clever reason for that? Or is it some legacy thing (I’ve seen some todos about that in the code)?

Could you suggest me any easy way to refactor destroying code into API+service way?


Replies (6)

RE: Delete work package action doesn't use ::WorkPackages::DestroyService - Added by Jens Ulferts over 7 years ago

Hi Dimitry,

just checked it myself and you are right, the WorkPackages::DestroyService is not used at the moment. I wasn’t aware of that myself.

At least when only deleting single work packages one should be able to use the service and in order to do that one would have to call DELETE /api/v3/work_packages/:id. The action already exists so it seems to be a negligence to not use the action. We are always interested in PRs :)

For deleting multiple work packages, it might be more work though. We put a lot of work into handling saving of the work packages more efficiently, which is why we are now using the services more often, but I’d guess that the service is not used yet when multiple work packages are deleted. Which is not to say that it wouldn’t be possible to do it.

Regards

Jens

RE: Delete work package action doesn't use ::WorkPackages::DestroyService - Added by Dmitry Kazakov over 7 years ago

Hi, Juns!

Thank you for your reply! I thing I managed to add calling DestroyService directly to the BulkController. It creates a bit of code duplication with the API code, but I have no idea how to change the frontend/routing to forward bulk-requests to API… Anyway, now I have almost finished the implementations of services for relations and only need to implement filtering by this extra column. I will make a pull request a bit later, when I’m finished.

Btw, what are the responsibilities of services in OpenProject? I can see that it is some Rails design pattern and in OpenProject they wrap database access into a transaction. Are there any other responsibilities on them?

And is there any documentation about writing unittests for OpenProject? I see some specs folder, which looks like a test, but I’m not sure about where to start…

(I’m sorry if my questions sound a bit dumb, I’m a C developer most of the time)

RE: Delete work package action doesn't use ::WorkPackages::DestroyService - Added by Jens Ulferts over 7 years ago

Hi Dimitri,

a tad of duplication is probably unavoidable as long as we have different services for when destroying multiple work packages as compared to when destroying but one, so don’t stress yourself.

Services where introduced to shift towards having less business logic in the model layer. This means leaving the “Rails way” where you have the Active Record pattern baked into the model layer so you would normally have your business logic in there. But the models have become to burdened so we decided to move towards services.

One of the benefits of having a service object is having the ability to test business logic without db access as mocking the db layer does not change the object under test. Granted, tests become more complicated when mocking but they also become much faster which is an issue in rails especially. Please see the tests/specs of the destroy service for an example of service unit tests.

Regards

Jens

RE: Delete work package action doesn't use ::WorkPackages::DestroyService - Added by Dmitry Kazakov over 7 years ago

Hi, Jens!

Thank you for your clarifications about the services and tests! I’ve found some docs about RSpec, I’ll try to do some tests afterwards.

Right now I’m having a small issue with creating a query filter for this new boolean field. I have creates my own filter in a separate file (link below), it shows in the GUI, it filters the tasks, but I cannot save the query due to some model validations, and I cannot understand why… It seems like :boolean symbol is somehow involved.

The file with the filter:
https://paste.kde.org/p2jfih7ok

RE: Delete work package action doesn't use ::WorkPackages::DestroyService - Added by Dmitry Kazakov over 7 years ago

It looks like :boolean filter type is handled incorrectly by the frontend. When I just select the “Yes” or “No” in the filter GUI, the parameter passed to the backend is ‘t’ or ‘f’, but when I try to save a filter it passes ‘true’ or ‘false’…

I’m wondering, what is the correct value for allowed_values field of the filter?

RE: Delete work package action doesn't use ::WorkPackages::DestroyService - Added by Dmitry Kazakov over 7 years ago

Okay, I kind of made both new option and custom fields use boolean types as a means of transport between frontend and backend. I’m not sure if it is correct, but we’ll see later. Now tests…

  • (1 - 6/6)
Loading...