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

Adds Parsely as VIP Integration #4660

Merged
merged 68 commits into from
Aug 18, 2023

Conversation

mehmoodak
Copy link
Member

@mehmoodak mehmoodak commented Jul 5, 2023

Description

  • Adds parsely in the list of supported VIP integrations and integrate it using ParselyIntegration class.
  • In Integration class adds the functionality of activating integration based on the config file provided by VIP.

Changelog Description

Adds Parse.ly as VIP Integration

Pre-review checklist

  • This change works and has been tested locally (or has an appropriate fallback).
  • This change works and has been tested on a Go sandbox.
  • This change has relevant unit tests (if applicable).
  • This change uses a rollout method to ease with deployment (if applicable - especially for large scale actions that require writes).
  • This change has relevant documentation additions / updates (if applicable).
  • I've created a changelog description that aligns with the provided examples.

Pre-deploy checklist

  • VIP staff: Ensure any alerts added/updated conform to internal standards (see internal documentation).

Steps to Test

  1. git checkout vip-parsely-41/adds-wp-parsely-vip-integration
  2. Run the vip site locally using vip dev-env start --slug [YOUR_SLUG] (if no env is available then create it)
  3. Create integrations config file inside php container and place configs (we have to do it manually because as of now vip dev-env doesn't have support to auto create this folder/file while creating new envs)
  • docker exec -it [CONTAINER_NAME] /bin/sh
  • touch /wp/config/integrations-config/parsely-config.php and adds following content
    • parsely can be replaced with any other slug of the integration
    • You may have to restart vip dev-env for any change in file.
<?php

return array(
  'env' => array(
  	'status' => 'disabled',
  	'config' => array (
  		'site_id' => 'site.com',
  		'api_secret' => 'site_api_secret',
  		'metadata_secret' => 'site_metadata_secret',
  	)
  ),
  'network_sites' => array (
  	'1' => array (
  		'status' => 'disabled',
  		'config' => array (
  			'site_id' => 'site1.com',
  			'api_secret' => 'site_api_secret_1',
  			'metadata_secret' => 'site_metadata_secret_1',
  		)
  	),
  	'2' => array (
  		'status' => 'enabled',
  		'config' => array (
  			'site_id' => 'site2.com',
  			'api_secret' => 'site_api_secret_2',
  			'metadata_secret' => 'site_metadata_secret_2',
  		)
  	),
  )
);
  1. Verify integration enablement with different combinations of vip configs which in our case means wp-parsely plugin is activated with the SiteID and secrets which are provided by VIP (credentails support will be available on v3.9 and above).
  2. Apart from enabling the plugin via VIP config customer can also enables it by placing \Automattic\VIP\Integrations\activate( 'parsely' ); in plugin-loader.php file so better to also test this flow.

@mehmoodak mehmoodak self-assigned this Jul 5, 2023
@mehmoodak mehmoodak force-pushed the vip-parsely-41/adds-wp-parsely-vip-integration branch from 2da87c7 to 041ae78 Compare July 5, 2023 08:13
@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Merging #4660 (b106e71) into develop (f814916) will increase coverage by 0.31%.
The diff coverage is 89.10%.

@@              Coverage Diff              @@
##             develop    #4660      +/-   ##
=============================================
+ Coverage      28.67%   28.98%   +0.31%     
- Complexity      4661     4692      +31     
=============================================
  Files            274      276       +2     
  Lines          20638    20711      +73     
=============================================
+ Hits            5918     6004      +86     
+ Misses         14720    14707      -13     
Files Changed Coverage Δ
integrations/block-data-api.php 0.00% <0.00%> (ø)
tests/bootstrap.php 0.00% <ø> (ø)
wp-parsely.php 54.81% <ø> (ø)
vip-integrations.php 44.44% <45.45%> (+44.44%) ⬆️
integrations/integration.php 92.85% <90.00%> (-7.15%) ⬇️
integrations/parsely.php 92.30% <92.30%> (ø)
integrations/integration-vip-config.php 95.23% <95.23%> (ø)
integrations/integrations.php 100.00% <100.00%> (+4.54%) ⬆️

... and 4 files with indirect coverage changes

Copy link
Member Author

@mehmoodak mehmoodak left a comment

Choose a reason for hiding this comment

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

Added comments for easier review.

integrations/block-data-api.php Show resolved Hide resolved
integrations/block-data-api.php Show resolved Hide resolved
integrations/block-data-api.php Show resolved Hide resolved
integrations/integrations.php Show resolved Hide resolved
integrations/parsely.php Outdated Show resolved Hide resolved
@mehmoodak mehmoodak force-pushed the vip-parsely-41/adds-wp-parsely-vip-integration branch from eef8761 to 20fb066 Compare July 5, 2023 11:08
@mehmoodak mehmoodak force-pushed the vip-parsely-41/adds-wp-parsely-vip-integration branch from b4abc53 to 133e825 Compare July 5, 2023 13:47
@mehmoodak mehmoodak force-pushed the vip-parsely-41/adds-wp-parsely-vip-integration branch from 1f8c5ec to 6a6a13f Compare July 5, 2023 15:04
Copy link
Contributor

@ingeniumed ingeniumed left a comment

Choose a reason for hiding this comment

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

Your comments are wonderful, thanks for adding them in. I really like the renaming changes you've done, as well as the helper methods that you have added in. Awesome work 💯

integrations/integration.php Outdated Show resolved Hide resolved
integrations/integrations.php Outdated Show resolved Hide resolved
integrations/integrations.php Outdated Show resolved Hide resolved
@mehmoodak mehmoodak force-pushed the vip-parsely-41/adds-wp-parsely-vip-integration branch from ccf1783 to 0a34274 Compare July 6, 2023 11:27
@mehmoodak mehmoodak force-pushed the vip-parsely-41/adds-wp-parsely-vip-integration branch from e019653 to 1365d2f Compare July 6, 2023 12:34
integrations/integration-config.php Outdated Show resolved Hide resolved
integrations/integrations.php Outdated Show resolved Hide resolved
integrations/integrations.php Outdated Show resolved Hide resolved
integrations/integrations.php Outdated Show resolved Hide resolved
integrations/integrations.php Show resolved Hide resolved
integrations/parsely.php Outdated Show resolved Hide resolved
integrations/integrations.php Outdated Show resolved Hide resolved
integrations/integrations.php Outdated Show resolved Hide resolved
integrations/integration-config.php Outdated Show resolved Hide resolved
@mehmoodak
Copy link
Member Author

@mjangda all of the feedback has been incorporated please have a look again. Thanks

integrations/parsely.php Outdated Show resolved Hide resolved
Copy link
Member

@mjangda mjangda left a comment

Choose a reason for hiding this comment

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

Testing this and it looks good.

Added one suggestion on how to pass through the parsely credentials.

@mehmoodak mehmoodak enabled auto-merge (squash) August 18, 2023 12:26
@mehmoodak mehmoodak merged commit d780d93 into develop Aug 18, 2023
41 checks passed
@mehmoodak mehmoodak deleted the vip-parsely-41/adds-wp-parsely-vip-integration branch August 18, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants