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

Blog connector test implemented and connector updated. #1177

Merged
merged 6 commits into from
Jan 5, 2021

Conversation

kidunot89
Copy link
Contributor

@kidunot89 kidunot89 commented Sep 23, 2020

Works towards #1093 .
Fixes #1175
Fixes #1187

Summary checklist

  • wpmu_new_blog callback replaced with wp_initialize_site callback due to the deprecation of the wpmu_new_blog hook.
  • wp_initialize_site callback tested and passing.
  • wp_delete_site callback implemented, testing and passing.
  • wpmu_activate_blog callback tested and passing.
  • add_user_to_blog callback tested and passing.
  • remove_user_from_blog callback tested and passing.
  • update_blog status function and all connected hooks tested and passing.

@dero @kasparsd I updated the Blog connector log meta with more info. It know include the the blog name, url, and id.

@kidunot89 kidunot89 requested review from kasparsd and dero September 23, 2020 21:40
@kidunot89 kidunot89 changed the title Blog connector test implemented and connector updated. WIP Blog connector test implemented and connector updated. Sep 23, 2020
@kidunot89 kidunot89 self-assigned this Sep 24, 2020
'deleted'
);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dero @johnbillion This callback was added to resolve #1175. Please take a look.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @kidunot89 -- this callback will add an appropriate log entry, but AFAICT won't prevent the related "User removed" etc. entries from being logged, which can really clutter the log when a blog with dozens of users is removed.

I'm OK approving this PR as is, but I think we should keep #1175 open until we're able to bundle up those excessive individual log entries somehow. I'm thinking perhaps we find a way to just log "XYZ users were removed from the ABC site."?

What do you guys think?

@kidunot89 kidunot89 changed the title WIP Blog connector test implemented and connector updated. Blog connector test implemented and connector updated. Sep 29, 2020
Copy link
Member

@dero dero left a comment

Choose a reason for hiding this comment

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

@kidunot89 Good work here, thanks!

One question related to #1175

'deleted'
);
}

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @kidunot89 -- this callback will add an appropriate log entry, but AFAICT won't prevent the related "User removed" etc. entries from being logged, which can really clutter the log when a blog with dozens of users is removed.

I'm OK approving this PR as is, but I think we should keep #1175 open until we're able to bundle up those excessive individual log entries somehow. I'm thinking perhaps we find a way to just log "XYZ users were removed from the ABC site."?

What do you guys think?

@kidunot89 kidunot89 force-pushed the tests/blogs-connector branch from fc59f8b to ad7a424 Compare November 3, 2020 23:35
@kidunot89 kidunot89 requested a review from dero November 3, 2020 23:35
@kidunot89 kidunot89 added this to the 3.6.1 milestone Jan 4, 2021
@kidunot89 kidunot89 force-pushed the tests/blogs-connector branch from ad7a424 to 532ea36 Compare January 5, 2021 22:56
@kidunot89 kidunot89 merged commit 9a1dff0 into develop Jan 5, 2021
@kidunot89 kidunot89 deleted the tests/blogs-connector branch January 5, 2021 23:16
@kidunot89 kidunot89 mentioned this pull request Jan 12, 2021
10 tasks
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.

siteurl on integration data for Site create/update/delete Site deletion isn't logged
2 participants