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

Add Undo functionality #1301

Open
wants to merge 8 commits into
base: trunk
Choose a base branch
from
Open

Add Undo functionality #1301

wants to merge 8 commits into from

Conversation

obenland
Copy link
Member

@obenland obenland commented Feb 10, 2025

Fixes #1289

Proposed changes:

  • Moves get_activity() and get_author() methods to Outbox class, where they feel more appropriate. These methods take an outbox id.
  • Adds undo() method to Outbox class, that accepts an outbox item and creates an Undo activity based on it. It accounts for Create and Add activities requiring their own "undo" type.
  • Updates Base::transform_object_properties() and Base_Object::to_array() to not only discard null values but also empty values unless they're false.
  • Adds unit tests for the new functionality.

Other information:

  • Have you written new tests for your changes, if applicable?

Testing instructions:

  • It's not hooked in anywhere, so tests are probably most appropriate at this point?
  • The changes to Base::transform_object_properties() and Base_Object::to_array() are probably what has the highest potential for unintended consequences. Anything standing you there?

@obenland
Copy link
Member Author

Tests fail because the activity saved in Outbox items has tag and cc properties, and the Activity object doesn't?

@obenland obenland force-pushed the add/undo branch 2 times, most recently from 4a2fd82 to 8ad2832 Compare February 13, 2025 15:09
@obenland
Copy link
Member Author

@pfefferle Setting the default for cc to null actually results in it showing up in objects as cc:[null] since there are no more checks after set_audience() runs.

Not sure how you feel about defaulting to an empty array for it in Activity?

@obenland
Copy link
Member Author

New approach: Being more prescriptive than checking for isset() also removes these empty properties.

@obenland obenland marked this pull request as ready for review February 13, 2025 17:23
@obenland obenland requested a review from a team February 13, 2025 17:23
@obenland obenland self-assigned this Feb 13, 2025
Copy link
Contributor

@mattwiebe mattwiebe left a comment

Choose a reason for hiding this comment

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

This seems good to me.

Where, if anywhere, could we also expose this in the UI? Should we add a wp-cli command?

@obenland
Copy link
Member Author

I like that, yes!

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.

Feature Request: Add the ability to send out an "Undo" for a previous activity
2 participants