feat(app_center): revert a snap to previous local revision#1972
feat(app_center): revert a snap to previous local revision#1972ashuntu merged 2 commits intoubuntu:mainfrom
Conversation
|
Hey folks, can someone kindly review this pull request? @matthew-hagemann @d-loose @Feichtmeier Please let me know if you have any questions. Cheers. |
ec56ff1 to
23adfb1
Compare
|
Rebased PR and squashed commits fixing melos errors. |
ashuntu
left a comment
There was a problem hiding this comment.
Thanks for this PR. I've added some comments for things I noticed at first glance which will need to be addressed before going forward.
Thank you for taking a look at my submission @ashuntu . I have addressed your comments and put up another PR in snapd.dart and made the requested changes to this repo. Please let me know if you have any questions. Cheers. |
|
I see there are some CI formatting checks. I will fix those errors and repush. |
|
@ashuntu Do you think the Integration error might have been created by some changes I did? https://github.com/ubuntu/app-center/actions/runs/18080314471/job/51504261035?pr=1972#step:4:488 I didn't modify any other dependencies except the |
It's likely because you didn't also commit changes to |
Thank you. I will commit and push that. |
|
Looks like I have some formatting errors. Let me apply a fix and push the commit. |
|
@ashuntu I have fixed the formatting issues and pushed the changes. Can you kindly approve the workflow so that I can test the changes in the CI ? And Can you let me know if there are more changes required for this PR or the companion PR canonical/snapd.dart#132 that adds this functionality. |
|
Looks like a trailing comma is missing. https://github.com/ubuntu/app-center/actions/runs/19723085068/job/56841667566?pr=1972#step:6:72 I will update and repush. |
Done . |
13c1197 to
fcdc96f
Compare
## Summary This PR adds support for retrieving local snap revisions and reverting snaps to previous versions, addressing the need for snap revert functionality in App Center. You can find that in the Pull Request on App Center : ubuntu/app-center#1972 ## Issue/Card Fixes #1164 on ubuntu/app-center#1164 ## Changes ### New Classes - **`SnapLocalRevision`**: Represents a local revision of an installed snap with `revision`, `version`, and `active` fields - Handles revision typing robustly (int/num/string) using `_parseRevision` method - Supports parsing negative revisions (e.g., "x123" → -123) - Includes proper JSON serialization via freezed ### New Methods - **`SnapdClient.getLocalRevisions(name)`**: Retrieves all locally installed revisions for a snap - Uses `GET /v2/snaps?select=all&snaps={name}` endpoint - Returns `List<SnapLocalRevision>` with revision details - Handles errors gracefully by returning empty list - **`SnapdClient.revertSnap(name)`**: Reverts a snap to its previous revision - Uses `POST /v2/snaps/{name}` with `{"action":"revert"}` payload - Returns change ID for monitoring operation status - Throws `SnapdException` on error responses ### Testing - Extended `MockSnapdServer` to support new endpoints and actions - Added comprehensive unit tests covering: - Basic functionality for both methods - Revision type handling (int/num/string) - Error cases and edge conditions - Proper request/response formatting ### Documentation - Added detailed dartdoc comments with usage examples - Documented parameters, return values, and exceptions ## Usage Example ```dart // Get local revisions final revisions = await client.getLocalRevisions('firefox'); for (final revision in revisions) { print('Revision ${revision.revision}: ${revision.version} ${revision.active ? '(active)' : ''}'); } // Revert snap try { final changeId = await client.revertSnap('firefox'); final change = await client.getChange(changeId); print('Revert status: ${change.status}'); } catch (e) { print('Revert failed: $e'); } ``` ## Testing All existing tests continue to pass, and new tests for the feature added. ## Files Changed 1. `lib/src/snapd_client.dart` - Core implementation 2. `lib/src/snapd_client.freezed.dart` - Generated freezed code 3. `lib/src/snapd_client.g.dart` - Generated JSON serialization 4. `test/snapd_test.dart` - mock server updates
|
@d-loose I have fixed the lint errors and tested this locally and it works. The version wasn't properly being displayed on the App Center UI, commited and pushed fix for that as well. I am using |
|
We can get a version of snapd.dart published sometime next week most likely. |
Thank you for the update @ashuntu ! |
|
snapd.dart 0.7.4 was just released canonical/snapd.dart#138 |
Thank you for the update @ashuntu . I will update, test and repush in a couple of hours. |
788bfb1 to
1e55a9f
Compare
|
@ashuntu I have updated the newly published snapd.dart in |
|
All CI checks pass! |
ashuntu
left a comment
There was a problem hiding this comment.
This looks pretty good now, just some minor comments on tests and localization.
- Add revert button to snap page for snaps with previous local revisions - Implement getLocalRevisions() and hasPreviousRevision() in SnapdService - Add revert confirmation dialog with version details - Show local snap version instead of channel version in UI - Add localization strings for revert feature - Use snapd.dart 0.7.4 with revertSnap() support Fixes ubuntu#1164 Signed-off-by: Gajesh Bhat <gajeshbht@gmail.com>
b020fbf to
0c45983
Compare
|
Thank you @gajeshbhat for your contribution. Some feedback:
|
Signed-off-by: Gajesh Bhat <gajeshbht@gmail.com>
|
Updated PR description as it was severly out of date. |
|
It seems that reverting "essential" snaps like I would assume it just carries the same risk that reverting any other snap has, just that the conditions for reverting are a little more strict. I'm not sure if there are issues with the App Center itself being a snap and reverting one it depends on while it is running, though. I think the revert would just fail with an error like it does if a snap is still open. |
I think there are enough safeguards in snapd to prevent reverts that can cause issue. But if that's something that has to be looked at closely for the app center, it would be out of scope for this PR. We might want to create a seperate issue ticket for that. |
ashuntu
left a comment
There was a problem hiding this comment.
I think this looks pretty good.
I'd like more in-depth error reporting on failures in the future if possible, but that can be a future change.
Thank you @ashuntu . I will create an issue to add it to the backlog. |
Summary
Adds "Revert update" functionality to allow users to revert snaps to their previous local version.
What's changed
Screenshots
Snap Revert Workflow example
Cannot double revert and hide the revert entry altogether for snaps that cannot be reverted.
Cannot Revert a Snap without two versions or a Snap that has not been updated.
How to test
Checklist
Related PR's
snapd.dart PR revert wrapper in 0.7.4: canonical/snapd.dart#132