Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
- update_process_project_info
- update_calculate_project_data
- update_calculate_project_update_status
Steps to reproduce
Proposed resolution
Create UpdateCalculator service
public getProjectData
protected processProjectInfo
protected updateProjectStatus
Create an UpdateProject value object
Create an UpdateServerProjectInfo value object
Remaining tasks
Review
User interface changes
N/A
Introduced terminology
N/A
API changes
Data model changes
N/A
Release notes snippet
N/A
Issue fork drupal-3580705
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
nicxvan commentedComment #3
dwwNot duplicate, but definitely related to #3100110: Convert update_calculate_project_update_status() into a class. I haven’t had time or urgency to circle back to that issue. I sort of remember not fully agreeing with where it ended up, but there’s a lot of prior work in there that needs to either be refreshed and finished or at least considered before proceeding here.
My initial concern with this plan (and its siblings) is we already have some weird services that are sort of incomplete and interact with each other in “interesting” ways. I wasn't around core for a few years when folks started OOP-ifying this module. I don't love any of how it’s currently split up. I'd love to take some time (which I don't have in the next ~3 weeks) to really think about it and come up with a more wholistic plan for how we should organize all this. I'd rather do that before we start introducing new services.
Comment #4
nicxvan commentedWe will do our best not to make this worse, but we do want to deprecate and move these functions.
Comment #5
nicxvan commentedDidn't do deprecations for new parameters, not sure how to handle a final and private constructor either.
I also didn't inject key value expirable, still need to do that.
I didn't update the CR yet.
Comment #7
nicxvan commentedFor the private final classes I just added the new parameter.
I added the deprecation.
I also added a custom CR for this that needs updating.
Comment #8
nicxvan commentedThis is ready for review.
Comment #9
nicxvan commentedStrict types is causing a couple of failures, I can't review why right now though.
Comment #10
nicxvan commentedI reached out to @godotislate in slack for help and here helped be find the cause in the build tests.
It turns out it's actually pretty deep in the extension so not something to fix here.
@godotislate said it would be OK to drop strict types and leave a comment.
Edit: definitely need to give @godotislate credit, but I'm on my phone and not logged into new.d.o
Comment #11
nicxvan commentedGot a few things to address, thanks!
Comment #12
nicxvan commentedOk I addressed most of the feedback, we still need to introduce the value object for project information.
Comment #14
nicxvan commentedOk I pushed the first value object, it introduced some failures though to track down.
Comment #16
nicxvan commentedI added the updateproject value object, one of the logic branches changed and I haven't tracked it down yet.
Comment #17
nicxvan commentedComment #20
nicxvan commentedOk I think this is ready for review, I created two value objects for the main method.
This issue is large, but we only touch 3 functions so hopefully it's ok, we add two value objects to help with the boundary of project comparison. The only logic changes were converting from an array to an object.
I took inspiration from the issue mentioned in 3, but kept the UpdateProject as a strict value object.
This could use further refactoring, but that should be done in follow ups.