Skip to content
This repository has been archived by the owner on Jul 12, 2024. It is now read-only.

Add the ability to create custom plugin builds #3044

Merged
merged 2 commits into from
Oct 17, 2019

Conversation

justinshreve
Copy link
Collaborator

Part of #3038.

This PR adds the ability to generate custom plugin builds. We would like to start testing onboarding with others inside of Automattic, including non-developers. Being able to generate and provide a zip so testers don't need to build development will increase testing adoption.

I wanted to make this more general purpose, so the build:release command now accepts some optional arguments: --slug and --features. Base feature flags are pulled from config/plugin.json and your additional changes are overlaid on top. When the build is complete, a woocommerce-admin-$slug.zip file will be generated.

For example, to create a woocommerce-admin-onboarding.zip build by enabling onboarding in addition to the feature flags defined in config/plugin.json, the command would be:

npm run build:release -- --slug onboarding --features '{"onboarding":true}'.

This PR also sets the default calypso environment to wpcalypso for now, so that Calypso changes can also easily be tested without needing to build a local copy of Calypso: Automattic/wp-calypso#36790

Detailed test instructions:

  • Test running the default npm run build:release command and verify things look as they should.
  • Test running npm run build:release -- --slug onboarding --features '{"onboarding":true}'. Uploading the resulting zip on another test site (such as wpsandbox) should allow you to test onboarding. You may need to reset the wizard under Orders > Help > Setup Wizard.

@psealock
Copy link
Collaborator

psealock commented Oct 15, 2019

The plugin build is not rendering the dashboard. Its unrelated to the changes here, but I'll bring it up here and separate it out to another issue if need be.

This is only reproduced using the plugin build, not development. Could there be a feature flag issue?

Screen Shot 2019-10-16 at 9 58 54 AM

profileItems is not defined

Screen Shot 2019-10-16 at 9 58 39 AM

Which is causing problems here:

visible: profileItems.items_purchased && ! profileItems.wccom_connected,

@justinshreve
Copy link
Collaborator Author

@psealock Can you upload the resulting .zip file somewhere? Seems like maybe the PHP side of the feature flags isn't loading / prefetching some of the data.

@psealock
Copy link
Collaborator

Sure. Shouldn't the task list logic not run at all if the onboarding flag is set to false for the plugin?

@justinshreve
Copy link
Collaborator Author

Oh sorry, I misunderstood. When you said this was unrelated to the changes, did you mean this is happening on master with the normal build plugin command? I’ll take a look tomorrow.

@justinshreve justinshreve changed the base branch from master to fix/onboarding-dashboard-plugin October 16, 2019 14:16
@justinshreve
Copy link
Collaborator Author

@psealock This has been fixed in #3053, and I've rebased this PR against it.

@psealock
Copy link
Collaborator

did you mean this is happening on master with the normal build plugin command?

Yes, thanks for addressing. I'll have another look at this one.

Copy link
Collaborator

@psealock psealock left a comment

Choose a reason for hiding this comment

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

When I build the plugin with onboarding turned on:

npm run build:release -- --slug onboarding --features '{"onboarding":true}'

The dashboard results in an error:

Screen Shot 2019-10-17 at 11 28 43 AM

A regular plugin build has no error and loads correctly. The dev environment with this branch works correctly too.

ZIP_FILE="woocommerce-admin-$2.zip";
fi
shift
done
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah ok, so the slug can be anything, ie onboarding-test-wpcom, but isn't required. Thats nice 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like the error reported here, #3056

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That was indeed the same error @psealock. I've rebased this, and made sure to test the zip with that option cleared out. Should be fixed now.

@justinshreve justinshreve changed the base branch from fix/onboarding-dashboard-plugin to master October 17, 2019 14:43
Copy link
Contributor

@jeffstieler jeffstieler left a comment

Choose a reason for hiding this comment

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

Code looks good (with one optional nitpick/question/comment) and tests well.

Tested on a Jurrasic test site, uploading the zips.

@@ -18,6 +18,14 @@
$config_json = file_get_contents( 'config/' . $phase . '.json' );
$config = json_decode( $config_json );

if ( ! empty( $_SERVER['WC_ADMIN_ADDITIONAL_FEATURES'] ) ) {
$additional_features = json_decode( $_SERVER['WC_ADMIN_ADDITIONAL_FEATURES'], true );
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to work fine, but would $_ENV or getenv() be better than $_SERVER here? ($_SERVER just makes me think we're in an HTTP request and not a cli script)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated to $_ENV. Agreed this makes more sense since they are environment variables.

4e49066.

@justinshreve justinshreve merged commit 38c6e92 into master Oct 17, 2019
@justinshreve justinshreve deleted the add/custom-plugin-builds branch October 17, 2019 18:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants