Problem/Motivation

Support farmOS 4 and Drupal 11

Issue fork farm_asset_termination-3569829

Command icon 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:

About issue forks

Comments

peacog created an issue. See original summary.

peacog’s picture

peacog commented
Version: 1.2.2 » 1.x-dev

peacog opened merge request !5

peacog’s picture

peacog commented
Status: Active » Needs review
m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs review » Needs work

Thanks @peacog!

Two things:

1. This changes the AssetTerminationFormatter annotation to an attribute, which was introduced in Drupal 10.3 (https://www.drupal.org/node/3229001), so we'll need to drop support for Drupal 9 and increase the minimum version to 10.3 (but see point #2 below first...)

2. farmOS 4.x recently merged a breaking change that merged the farm_settings module into farm_setup, which also renamed the farm_settings.settings_page base route to farm.setup.settings (see: https://github.com/farmOS/farmOS/pull/1024 + https://www.drupal.org/node/3559903).

So we'll need to change:

farm_asset_termination.settings:
  base_route: farm_settings.settings_page
  ...

to:

farm_asset_termination.settings:
  base_route: farm.setup.settings
  ...

And because farm.setup.settings only exists in farmOS 4.x+, we'll need to drop support for Drupal 10 / farmOS v3.

With all of that in mind, it might make sense to tag a new major version after this is tested and merged. I'll leave that decision up to @wotnak.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs work » Needs review

Pushed two commits, which drop support for Drupal 9 and 10, and update the settings base_route.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented

Ah we also need to update AssetTerminationSettingsForm to inject TypedConfigManagerInterface, which is a new dependency of ConfigFormBase. Pushed another commit that adds this.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs review » Needs work

Testing the batch operations and they appear to be broken... not sure if it's related to farmOS v4 specifically, but I'll try to fix them here.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs work » Needs review

Fixed!

@wotnak let me know if you would prefer a separate issue for the batch operations fix. Otherwise I think this is ready for review.

wotnak’s picture

wotnak
he/him
commented
Status: Needs review » Reviewed & tested by the community

  • wotnak committed 931ec3d3 on 2.x authored by peacog 
    feat: #3569829 Support farmOS 4

By: peacog
By: m.stenta
By: wotnak
wotnak’s picture

wotnak
he/him
commented
Status: Reviewed & tested by the community » Fixed

Released in v2.0.0.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.