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.

Sign up for GitHub

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

Jump to bottom

Call setup_new_site() upon plugin activation #7489

Merged
merged 2 commits into from
May 12, 2021
Merged

Call setup_new_site() upon plugin activation #7489

merged 2 commits into from
May 12, 2021

Conversation

swissspidy
Copy link
Collaborator

Context

Site setup logic was not being run during plugin activation

Summary

Adds back the setup_new_site() call we had here previously.

Relevant Technical Choices

To-do

  • Investigate why e2e tests didn't catch this.

User-facing changes

Testing Instructions

QA

  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

  1. Install plugin on a fresh site (locally, or Jurassic Ninja or similar)
  2. Activate plugin
  3. Verify the "Stories" admin menu item is visible
  4. Verify the DB upgrader did run by checking the DB version in wp_options / wp-admin/options.php

UAT

  • UAT should use the same steps as above.

This PR can be tested by following these steps:

Reviews

Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This PR contains automated tests (unit, integration, and/or e2e) to verify the code works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #7488

@swissspidy swissspidy added Type: Bug Something isn't working Pod: WP & Infra labels May 11, 2021
@swissspidy swissspidy requested a review from spacedmonkey May 11, 2021 18:43
@google-cla google-cla bot added the cla: yes label May 11, 2021
@github-actions
Copy link
Contributor

github-actions bot commented May 11, 2021 •
edited
Loading

Size Change: +3.23 kB (0%)

Total Size: 2.12 MB

Filename Size Change
assets/js/edit-story.js 517 kB +1.08 kB (0%)
assets/js/stories-dashboard.js 434 kB +1.19 kB (0%)
assets/js/web-stories-block.js 583 kB +951 B (0%)
ℹ️ View Unchanged
Filename Size Change
assets/css/carousel-view-rtl.css 715 B 0 B
assets/css/carousel-view.css 716 B 0 B
assets/css/edit-story-rtl.css 278 B 0 B
assets/css/edit-story.css 278 B 0 B
assets/css/stories-dashboard-rtl.css 276 B 0 B
assets/css/stories-dashboard.css 276 B 0 B
assets/css/vendors-edit-story-rtl.css 702 B 0 B
assets/css/vendors-edit-story.css 702 B 0 B
assets/css/web-stories-block-rtl.css 3.23 kB 0 B
assets/css/web-stories-block.css 3.26 kB 0 B
assets/css/web-stories-embed-rtl.css 284 B 0 B
assets/css/web-stories-embed.css 284 B 0 B
assets/css/web-stories-list-styles-rtl.css 2.26 kB 0 B
assets/css/web-stories-list-styles.css 2.27 kB 0 B
assets/css/web-stories-theme-style-twentyeleven-rtl.css 102 B 0 B
assets/css/web-stories-theme-style-twentyeleven.css 102 B 0 B
assets/css/web-stories-theme-style-twentyfifteen-rtl.css 252 B 0 B
assets/css/web-stories-theme-style-twentyfifteen.css 252 B 0 B
assets/css/web-stories-theme-style-twentyfourteen-rtl.css 286 B 0 B
assets/css/web-stories-theme-style-twentyfourteen.css 286 B 0 B
assets/css/web-stories-theme-style-twentyseventeen-rtl.css 311 B 0 B
assets/css/web-stories-theme-style-twentyseventeen.css 311 B 0 B
assets/css/web-stories-theme-style-twentysixteen-rtl.css 239 B 0 B
assets/css/web-stories-theme-style-twentysixteen.css 239 B 0 B
assets/css/web-stories-theme-style-twentyten-rtl.css 144 B 0 B
assets/css/web-stories-theme-style-twentyten.css 145 B 0 B
assets/css/web-stories-theme-style-twentytwelve-rtl.css 263 B 0 B
assets/css/web-stories-theme-style-twentytwelve.css 263 B 0 B
assets/css/web-stories-theme-style-twentytwenty-rtl.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwenty.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwentyone-rtl.css 263 B 0 B
assets/css/web-stories-theme-style-twentytwentyone.css 265 B 0 B
assets/css/web-stories-widget-rtl.css 489 B 0 B
assets/css/web-stories-widget.css 489 B 0 B
assets/js/carousel-view.js 3.72 kB 0 B
assets/js/chunk-fonts-********************.js 45.4 kB 0 B
assets/js/chunk-web-stories-template-0-********************.js 7.01 kB 0 B
assets/js/chunk-web-stories-template-1-********************.js 8.58 kB 0 B
assets/js/chunk-web-stories-template-2-********************.js 7.89 kB 0 B
assets/js/chunk-web-stories-template-3-********************.js 8.68 kB 0 B
assets/js/chunk-web-stories-template-4-********************.js 9.76 kB 0 B
assets/js/chunk-web-stories-template-5-********************.js 6.08 kB 0 B
assets/js/chunk-web-stories-template-6-********************.js 8.13 kB 0 B
assets/js/chunk-web-stories-template-7-********************.js 7.89 kB 0 B
assets/js/chunk-web-stories-textset-0-********************.js 5.29 kB 0 B
assets/js/chunk-web-stories-textset-1-********************.js 6.87 kB 0 B
assets/js/chunk-web-stories-textset-2-********************.js 7.92 kB 0 B
assets/js/chunk-web-stories-textset-3-********************.js 15.4 kB 0 B
assets/js/chunk-web-stories-textset-4-********************.js 4.43 kB 0 B
assets/js/chunk-web-stories-textset-5-********************.js 5.71 kB 0 B
assets/js/chunk-web-stories-textset-6-********************.js 5.5 kB 0 B
assets/js/chunk-web-stories-textset-7-********************.js 10.4 kB 0 B
assets/js/lightbox.js 986 B 0 B
assets/js/tinymce-button.js 3.48 kB 0 B
assets/js/vendors-chunk-ffmpeg-********************.js 5.61 kB 0 B
assets/js/vendors-edit-story-********************.js 61.4 kB 0 B
assets/js/vendors-edit-story-stories-dashboard-********************.js 258 kB +1 B (0%)
assets/js/web-stories-activation-notice.js 65.1 kB 0 B
assets/js/web-stories-embed.js 493 B 0 B
assets/js/web-stories-widget.js 984 B 0 B

compressed-size-action

spacedmonkey
spacedmonkey approved these changes May 11, 2021
View reviewed changes
Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

👍

@spacedmonkey
Copy link
Contributor

Wow, I don't know how I missed this. Instead of needing this function, I have these two todos.

https://github.com/google/web-stories-wp/blob/ee98d46ddfec1c1224ca450980e3d896aa8c4678/includes/namespace.php#L47

https://github.com/google/web-stories-wp/blob/ee98d46ddfec1c1224ca450980e3d896aa8c4678/includes/namespace.php#L53

I will create a follow up ticket. Sorry I didn't do this earlier.

@spacedmonkey
Copy link
Contributor

Seems like i already did some of this work.

https://github.com/google/web-stories-wp/blob/ee98d46ddfec1c1224ca450980e3d896aa8c4678/includes/Database_Upgrader.php#L104-L112

https://github.com/google/web-stories-wp/blob/ee98d46ddfec1c1224ca450980e3d896aa8c4678/includes/User/Capabilities.php#L40-L48

@spacedmonkey
Copy link
Contributor

Looking at this closely, I think that this would be a better solution.

@swissspidy
Copy link
Collaborator Author

We need something simple now; #7494 can be looked at after release

@swissspidy
Copy link
Collaborator Author

Seems like i already did some of this work.

https://github.com/google/web-stories-wp/blob/ee98d46ddfec1c1224ca450980e3d896aa8c4678/includes/Database_Upgrader.php#L104-L112

Only runs on admin_init, so never executed in a WP-CLI context.

https://github.com/google/web-stories-wp/blob/ee98d46ddfec1c1224ca450980e3d896aa8c4678/includes/User/Capabilities.php#L40-L48

Not a service, so this method will never be run

@swissspidy swissspidy merged commit 20f69bd into main May 12, 2021
@swissspidy swissspidy deleted the fix/7488-plugin-activation branch May 12, 2021 07:56
@spacedmonkey spacedmonkey mentioned this pull request May 12, 2021
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spacedmonkey spacedmonkey spacedmonkey approved these changes

Assignees
No one assigned
Labels
Type: Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Site Setup logic not run uon plugin activation
2 participants
@swissspidy @spacedmonkey