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

Make commit_reversed_operations more capable. #427

Open
djmitche opened this issue Jul 21, 2024 · 0 comments
Open

Make commit_reversed_operations more capable. #427

djmitche opened this issue Jul 21, 2024 · 0 comments
Labels
good first issue Good for newcomers

Comments

@djmitche
Copy link
Collaborator

Right now, this method (known as commit_undo_ops before #372) just checks that the latest set of un-synchronized operations in the replica match the operations to undo. If that's the case, it removes them from the list of un-synchronized operations, reverses their effect on the tasks, and declares success. Otherwise, it fails.

This could be more robust -- if an operation does not precisely match the most recently recorded local operation, there's enough data to create a new, reversed operation (switch Create <--> Delete, swap value and old_value in Update) and commit that. Adding such support would be a big step on the way to allowing "undo" after a sync has occurred.

I think we should fail the commit_reversed_operations if the reversed operation doesn't apply cleanly. So:

  • A reversed Create is a Delete, but if the task doesn't exist, that should fail.
  • A reversed Delete is a Create, but if the task still exists, that should fail.
  • A reversed Update specifies the old_value it expects to find, but if the value is different, it should fail.
djmitche added a commit to djmitche/taskchampion that referenced this issue Jul 21, 2024
@djmitche
[BREAKING] Update undo support to use Operations
f977e40 
This updates the existing "undo" support to use `Operations`, while
making some additional changes to look forward to more general undo
support (GothenburgBitFactory#427 being the next step on that journey).

Breaking changes:

 - `Replica::get_undo_ops` is now `Replica::get_undo_operations` and returns an
   `Operations` value. It now returns the operations in the order they were
   applied, and includes the undo point operation, if one exists.
 - `Replica::commit_undo_ops` is now `Replica::commit_reversed_operations` and
   takes an `Operations` value. It now expects an `Operations` value as
   provided by `get_undo_operations`.
djmitche added a commit to djmitche/taskchampion that referenced this issue Jul 25, 2024
@djmitche
[BREAKING] Update undo support to use Operations
fceb45e 
This updates the existing "undo" support to use `Operations`, while
making some additional changes to look forward to more general undo
support (GothenburgBitFactory#427 being the next step on that journey).

Breaking changes:

 - `Replica::get_undo_ops` is now `Replica::get_undo_operations` and returns an
   `Operations` value. It now returns the operations in the order they were
   applied, and includes the undo point operation, if one exists.
 - `Replica::commit_undo_ops` is now `Replica::commit_reversed_operations` and
   takes an `Operations` value. It now expects an `Operations` value as
   provided by `get_undo_operations`.
djmitche added a commit to djmitche/taskchampion that referenced this issue Jul 26, 2024
@djmitche
[BREAKING] Update undo support to use Operations
56c6a22 
This updates the existing "undo" support to use `Operations`, while
making some additional changes to look forward to more general undo
support (GothenburgBitFactory#427 being the next step on that journey).

Breaking changes:

 - `Replica::get_undo_ops` is now `Replica::get_undo_operations` and returns an
   `Operations` value. It now returns the operations in the order they were
   applied, and includes the undo point operation, if one exists.
 - `Replica::commit_undo_ops` is now `Replica::commit_reversed_operations` and
   takes an `Operations` value. It now expects an `Operations` value as
   provided by `get_undo_operations`.
djmitche added a commit to djmitche/taskchampion that referenced this issue Jul 29, 2024
@djmitche
[BREAKING] Update undo support to use Operations
83566d9 
This updates the existing "undo" support to use `Operations`, while
making some additional changes to look forward to more general undo
support (GothenburgBitFactory#427 being the next step on that journey).

Breaking changes:

 - `Replica::get_undo_ops` is now `Replica::get_undo_operations` and returns an
   `Operations` value. It now returns the operations in the order they were
   applied, and includes the undo point operation, if one exists.
 - `Replica::commit_undo_ops` is now `Replica::commit_reversed_operations` and
   takes an `Operations` value. It now expects an `Operations` value as
   provided by `get_undo_operations`.
djmitche added a commit to djmitche/taskchampion that referenced this issue Jul 29, 2024
@djmitche
[BREAKING] Update undo support to use Operations
6370b45 
This updates the existing "undo" support to use `Operations`, while
making some additional changes to look forward to more general undo
support (GothenburgBitFactory#427 being the next step on that journey).

Breaking changes:

 - `Replica::get_undo_ops` is now `Replica::get_undo_operations` and returns an
   `Operations` value. It now returns the operations in the order they were
   applied, and includes the undo point operation, if one exists.
 - `Replica::commit_undo_ops` is now `Replica::commit_reversed_operations` and
   takes an `Operations` value. It now expects an `Operations` value as
   provided by `get_undo_operations`.
@djmitche djmitche mentioned this issue Jul 29, 2024
Merged
djmitche added a commit that referenced this issue Jul 29, 2024
@djmitche
[BREAKING] Update undo support to use Operations (#437)
ecb7a20 
This updates the existing "undo" support to use `Operations`, while
making some additional changes to look forward to more general undo
support (#427 being the next step on that journey).

Breaking changes:

 - `Replica::get_undo_ops` is now `Replica::get_undo_operations` and returns an
   `Operations` value. It now returns the operations in the order they were
   applied, and includes the undo point operation, if one exists.
 - `Replica::commit_undo_ops` is now `Replica::commit_reversed_operations` and
   takes an `Operations` value. It now expects an `Operations` value as
   provided by `get_undo_operations`.
@djmitche djmitche added the good first issue Good for newcomers label Aug 5, 2024
@djmitche djmitche mentioned this issue Aug 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
Taskwarrior Development
Status: Backlog
Milestone
No milestone
Development

No branches or pull requests
1 participant
@djmitche