Skip to content

Conversation

@justlevine
Copy link
Collaborator

What does this implement/fix? Explain your changes.

This PR refactors our PluginConnectionResolver class to use a DRY ::get_all_plugins() method for building the list of available plugins.
This list is then stored in a class property, so the fetching, filtering, etc only occur once.

Does this close any currently open issues?

Any relevant logs, error output, GraphiQL screenshots, etc?

Part of #2749

Any other comments?

This PR is based on #3082 , which should be merged first.

Where has this been tested?

Operating System: Ubuntu 20.04 (wsl2 + devilbox + php 8.1.15)

WordPress Version: 6.4.3

@justlevine justlevine requested a review from jasonbahl March 30, 2024 13:24
@justlevine justlevine added type: enhancement Improvements to existing functionality status: in review Awaiting review before merging or closing needs: reviewer response This needs the attention of a codeowner or maintainer component: connections Relating to GraphQL Connections object type: plugin Relating to the Plugin Type labels Mar 30, 2024
@jasonbahl jasonbahl merged commit 7857764 into chore/connection-resolvers-cleanup Apr 1, 2024
@justlevine
Copy link
Collaborator Author

justlevine commented Apr 1, 2024

Ps @jasonbahl it might be easier for you to review/merge #3082 first, (PRs based on it, should auto-update the branch to develop) instead of merging the dependent PRs into #3082.

From my side: I don't know your availability for reviewing them all, but assuming it's a multi-day process, then merging the parent PRs before the dependents would also let me continue backporting more things while decreasing the risk of merge conflicts.

Either way

@justlevine justlevine deleted the dev/cr2-backport/refactor-plugin-connection-resolver branch April 1, 2024 21:20
@jasonbahl
Copy link
Collaborator

This one didn't look like it had any direct connection to the others and could be merged as-is.

The others did seem to have a lot more correlation.

@justlevine
Copy link
Collaborator Author

@jasonbahl this was merged into #3082 , not into #3097 . We probably should amend the release notes.

@jasonbahl
Copy link
Collaborator

@justlevine ooph. Good call. Will update. Thanks. Just got the test results back and this was another case of the "moving too fasts"

@jasonbahl
Copy link
Collaborator

@justlevine edited the release notes: https://github.com/wp-graphql/wp-graphql/releases/tag/v1.23.0

This was referenced Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: connections Relating to GraphQL Connections needs: reviewer response This needs the attention of a codeowner or maintainer object type: plugin Relating to the Plugin Type status: in review Awaiting review before merging or closing type: enhancement Improvements to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants