Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update drupal/migrate_plus requirement from 6.0.2 to 6.0.4 #3555

Merged
merged 5 commits into from
Jul 24, 2024

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Jul 18, 2024

Updates the requirements on drupal/migrate_plus to permit the latest version.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

Updates the requirements on drupal/migrate_plus to permit the latest version.

---
updated-dependencies:
- dependency-name: drupal/migrate_plus
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot added the dependencies Pull requests that update a dependency file label Jul 18, 2024
@dependabot dependabot bot requested a review from a team as a code owner July 18, 2024 23:00
@dependabot dependabot bot added the php Pull requests that update Php code label Jul 18, 2024
@bberndt-uaz
Copy link
Contributor

https://www.drupal.org/project/migrate_plus/releases/6.0.3

Release notes

Various fixes and enhancements

Changes since 6.0.2:

Bug

Feature

Task

@joeparsons
Copy link
Member

Looks like this patch release contains a backwards-incompatible API change 😞:

------ --------------------------------------------------------------------------------------------------------------------------- 
 Line   modules/custom/az_core/az_enterprise_attributes_import/src/Plugin/migrate_plus/data_parser/AZEnterpriseAttributesJson.php  
------ --------------------------------------------------------------------------------------------------------------------------- 
  20     Method                                                                                                                     
         Drupal\az_enterprise_attributes_import\Plugin\migrate_plus\data_parser\AZEnterpriseAttributesJson::getSourceData()         
         overrides method                                                                                                           
         Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Json::getSourceData()                                                  
         but misses parameter #2 $item_selector.                                                                                    
  21     Method                                                                                                                     
         Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Json::getSourceData()                                                  
         invoked with 1 parameter, 2 required.                                                                                      
 ------ --------------------------------------------------------------------------------------------------------------------------- 


 [ERROR] Found 2 errors              

@joeparsons joeparsons added Drupal Waiting on a fix from the Drupal community 2.11.x only labels Jul 19, 2024
@joeparsons
Copy link
Member

joeparsons commented Jul 19, 2024

Created an issue and MR on drupal.org about the backwards-incompatible changes to the Json data_parser :
https://www.drupal.org/project/migrate_plus/issues/3462520

@tadean
Copy link
Contributor

tadean commented Jul 19, 2024

There have been some significant changes in this release to the migrate_plus Json parser, and some changes to the Http fetcher. I think this warrants some thorough review to make sure that any methods we're overriding are still correct.

We should also review the classes which override the Http fetcher for whether we need to update those classes (AzHttp and RetryHttp)

My testing on this PR also uncovered a seemingly unrelated new issue in 2.11:
#3556

@tadean
Copy link
Contributor

tadean commented Jul 23, 2024

Merged with main to bring in changes from #3558

Verified that:

  • Enterprise attributes import still function, and terms are nested with parents
  • Enterprise attributes still have attribute keys
  • Courses still import
  • Publication DOI import still works
  • Trellis events still import
  • The above also all pass with az_http enabled

After the changes @joeparsons made to the function signature of AZEnterpriseAttributesJson to match the parent, it looks like the class is unaffected by the other changes made to the parent class. I think this is probably because the function calls the parent implementation already, and just changes the data after the fact.

tadean
tadean previously approved these changes Jul 23, 2024
Copy link
Contributor Author

dependabot bot commented on behalf of github Jul 23, 2024

A newer version of drupal/migrate_plus exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.

@joeparsons
Copy link
Member

joeparsons commented Jul 23, 2024

https://www.drupal.org/project/migrate_plus/releases/6.0.4 is now available as well. It just includes the fix we contributed for the issue we reported:

Fix backwards-incompatible Json data_parser API change included in 6.0.3.

@tadean Do you think we ought to consider using 6.0.4 and revert the changes to our plugin class or would you rather keep the changes to match the parent?

@joeparsons joeparsons changed the title Update drupal/migrate_plus requirement from 6.0.2 to 6.0.3 Update drupal/migrate_plus requirement from 6.0.2 to 6.0.4 Jul 23, 2024
@tadean
Copy link
Contributor

tadean commented Jul 24, 2024

@tadean Do you think we ought to consider using 6.0.4 and revert the changes to our plugin class or would you rather keep the changes to match the parent?

I'm in favor of keeping the changes, since we're overriding the class. I think it makes sense to keep the method matching the parent's signature, even if the 6.0.4 changes make the API change 'opt-in.'

@tadean
Copy link
Contributor

tadean commented Jul 24, 2024

Retested with migrate_plus version 6.0.4 with the same steps above.

@joeparsons joeparsons merged commit 383e443 into main Jul 24, 2024
16 checks passed
@joeparsons joeparsons deleted the dependabot/composer/drupal/migrate_plus-6.0.3 branch July 24, 2024 18:47
joeparsons added a commit that referenced this pull request Jul 24, 2024
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joe Parsons <[email protected]>
Co-authored-by: tadean <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.11.x only dependencies Pull requests that update a dependency file Drupal Waiting on a fix from the Drupal community php Pull requests that update Php code
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants