Drupal 10 compatibility

matthew.hallsworth.dpc → made their first commit to this issue’s fork.

I've put up a MR to bring this module in line with D10.

- D10 upgrade checker runs over this branch and gives it no issues
- Module installs correctly once other required modules are installed
- Can create a functional config in the UI

Issues as far as I can tell:

- Module has a namespace / machine name mismatch - the module's Drupal machine name is `section_purger` and all the namespaces are `section_purger`, but the module is on D.O. as `section_purge` and the composer install library name is `section_purge`. This is causing big issues when doing such things as automatic module loading in DrupalPod etc. and using PHPUnit. This probably needs fixing
- The test suites (either because of above, or other reasons) refuse to run even in a proper D10 install, due to 'file not found' issues thanks to namespacing.
- Since this module requires a Section.IO account to test, it's difficult to give it a functional test. I am investigating if it is possible to build some PHPUnit mocks for it to test the internal POST requests of the purge methods currently. If I can figure that out, I will add them to this branch.

I would like someone to be able to test this module but it may be difficult. Code review can be done however.

Thanks!

Testing moving this out and back into "Needs review" now that the tests have been configured per 📌 Add Drupal.org CI testing for section_purge Fixed .

Back to needs review to see if it kicks off tests.

Didn't work afaict... guess I have to actually read the docs :D

Chatting with hestenet and cmlara about this in the #drupalorg channel... one suggestion was to try another commit in the MR and another was to try moving to RTBC so I will try the latter right now.

That didn't seem to do anything afaict so changing status back.

Considering the latest MR. the module worked well in d10

We are still trying to get tests to work but please explain in detail how you tested, thanks.

Thanks to @cmlara for the help on this issue!

Kristen Pol → credited cmlara → .

Triggering a retest and a test on 10.0.

@matthew.hallsworth.dpc @malks I noticed that there are TONS of changes in the MR that seem to just to be some formatting change/glitch:

https://git.drupalcode.org/project/section_purge/-/merge_requests/3/diffs

I didn't look in detail but we need to make sure we can diff this properly and only have the actual changes shown.

Going to try a patch from the PR since I can't seem to get it working for the life of me.

This patch includes all PHPCS fixes so lets see if we can get past the composer issue this way.

Diff was bad, going to try another one to get to the bottom of this.

@rckstr_rohan would you be able to give the latest code in the MR another test and leave some dot points in your response as to what you tested and what your outcomes were?

Thanks a lot!

I think comment #5 has already pointed out an issue which may explain why MR's are failing.

- Module has a namespace / machine name mismatch - the module's Drupal machine name is `section_purger` and all the namespaces are `section_purger`, but the module is on D.O. as `section_purge` and the composer install library name is `section_purge`. This is causing big issues when doing such things as automatic module loading in DrupalPod etc. and using PHPUnit. This probably needs fixing

Without looking in depth to the buildbot code or running an actual test:
I think that may explain what is going on with merge requests failing to require the module. When the git branch is checked out the composer.json indicates the code is for a module called 'drupal/section_purger', if composer reads that file it may change any association internally from drupal/section_purge into it being drupal/section_purger, which could cause require drupal/section_purge to fall back to looking to the D.O. package server for the merge request branch which does not (and should not) exist.

Recommendations really are to follow the comments from #5, at a minimal this means:
Change drupal/section_purger to drupal/section_purge in composer.json
Change namespace in all files to section_purge.
Try and re-run the tests.
Contact D.O. infrastructure team and have them remove the package server entry for drupal/section_purger (if they can, it actually might be tagged to a few releases causing an issue)

Oh dear! This was set up before my time here… certainly okay with naming changes as I like consistency but am worried about the effect on people already using the module. I’ll check with the infrastructure team.

Can a new development branch be created as a breaking change, for D10 only, to rename all the namespaces and module names to match the composer name? This could be 4.x-dev and be D10 only. The branch I have currently can be changed to base off this.

That way, existing installs won't be affected, as part of the upgrade cycle they would have to consider the changes and deal with it accordingly.

As it is, the existing branches don't really work for D9 - we use a branch as the current install on our D9 installs anyway.

Thoughts?

Nice working getting tests green! :)

We need two things:

1. Code review

2. Manual testing including an explanation of how it was tested

Thanks!

@rckstr_rohan are you able to re-test the current MR and provide testing steps that you performed?

Thanks!

starting to work on it, will perform the test to the latest MR today

@rckstr_rohan any updates?

Hi, i have some eye dryness infesction due to laptop, mobile rays, so keeping myself away for 10 days as instructed by doctor, will surly work on it after a week, thanks.

@rckstr_rohan sorry to hear about your health. Have you had a chance to catch up on this testing?

will proceed now, have done some in past will complete and update, thanks.

Install and configured the module with instance, looks good,but the section getting config failure, even I have configured the section.io through the docs. so may be the problem from section.io as tried multiple times.

the section config error

@rckstr_rohan

Thanks for the testing. That looks like it might be from section themselves, I can't see that error in the codebase.

I wonder if the API has changed?

Adding Patch for D10 Branch Diff

We have done some testing with the new changes with the changed module name and related settings from @matthew.hallsworth.dpc and it looks good, working properly. Here are couple of screenshot from the testing branch with section.

@Kristen Pol I'm making this RTBC and I'd like to work with you on a release for this with an eye to helping maintain the module moving forward.

Thanks!

Thanks, everyone, for moving this forward.

@matthew.hallsworth Would love to have the help. Please create a new issue (in this issue queue, not the projectownership queue) offering to co-maintain the module similar to this:

https://www.drupal.org/project/projectownership/issues/3344209 →

Feel free to ping me in Slack to move this forward. Thanks :)