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

Integrations: Extract into separate classes #345

Merged
merged 5 commits into from
Sep 21, 2021

Conversation

GaryJones
Copy link
Contributor

@GaryJones GaryJones commented Jun 24, 2021

Description

  • Extract code for AMP and Facebook Instant Article integrations into new classes in separate files, out of the Parsely class.
  • Improve and add tests that better check for the intended behaviour - adds 6 new tests and 19 new assertions.

It's the same behaviour for the end-user and site.

See #96.

Motivation and Context

The Parsely class does far too much, and needs to be split out into more logical classes to make it easier to maintain.

One area where this can be done is the extra code that makes the plugin work with other plugins i.e. integrations.

By creating a class for each integration, we logically group related code together, making it easier to see what integrations there are. The use of a collection and an interface (though this interface isn't yet checked for when registering an integration to the collection) provides a structure for built-in and other plugins to create their own integrations between Parse.ly and another plugin.

How Has This Been Tested?

AMP

  1. With the AMP plugin not active, the amp_* filters won't run, so there are no negative side effects.
  2. Install and activate the AMP plugin.
  3. Visit any post and add /amp to the end of the URL.
  4. Inside the source of the resulting page, look for a <amp-analytics opening tag that contains references to Parse.ly.

Screenshot 2021-09-18 at 16 28 33

Facebook Instant Articles

  1. With the FBIA plugin not active, the instant_articles_* filter won't run, so there are no negative side effects.
  2. Install and activate the Facebook Instant Articles plugin. Visit the Settings->Permalinks page to flush the rewrite rules.
  3. Visit the /wp-admin/admin.php?page=instant-articles-wizard admin page. If only one field is visible, enter any random string and save to then see many more settings.
  4. Look for the Analytics section, and enable the Parse.ly Analytics checkbox.
  5. Visit /feed/instant-articles and you should see XML. Look for the Parsely embed code from Parsely\Integrations\Facebook_Instant_Articles::get_embed_code().

Screenshot 2021-09-18 at 16 31 29

Screenshot 2021-09-18 at 16 29 56

Screenshots (if appropriate):

Coverage metrics for the new Integrations directory:

Screenshot 2021-09-18 at 17 06 03

Integrations directory coverage compared to the rest of the plugin:
Screenshot 2021-09-18 at 17 05 58

Integrations-related tests:

Screenshot 2021-09-18 at 17 18 13

Types of changes

  • Maintenance (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@GaryJones GaryJones self-assigned this Jun 24, 2021
@GaryJones GaryJones force-pushed the experimental/refactor-integrations branch 5 times, most recently from 5f5a944 to c830af5 Compare September 18, 2021 16:19
@GaryJones GaryJones marked this pull request as ready for review September 18, 2021 16:19
@GaryJones GaryJones requested a review from a team as a code owner September 18, 2021 16:19
Copy link
Contributor

@pauarge pauarge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome, it really improves the plugin structure! I left a minor question

src/Integrations/class-facebook-instant-articles.php Outdated Show resolved Hide resolved
- Move AMP and Facebook Instant Article integrations into separate files.
- Improve and add tests that better check for the intended behaviour.
@GaryJones GaryJones force-pushed the experimental/refactor-integrations branch from ebc4900 to 6d38b71 Compare September 20, 2021 16:24
GaryJones and others added 2 commits September 21, 2021 15:48
- Move AMP and Facebook Instant Article integrations into separate files.
- Improve and add tests that better check for the intended behaviour.
…ly/wp-parsely into experimental/refactor-integrations
@GaryJones GaryJones merged commit 4128196 into develop Sep 21, 2021
@GaryJones GaryJones deleted the experimental/refactor-integrations branch September 21, 2021 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants