feat: refresh multiple archives #1723

diivi · 2023-06-01T21:00:40Z

Description

First task for one of my GSoC Projects, "Enhancing the Archive Table actions". I listed these tasks out in the proposal, in total they should take me around 50 hours:

This will allow Vorta users to select multiple archives and refresh them at once, hence improving UX.

Motivation and Context

Saves the user's time.

How Has This Been Tested?

I tested this manually:

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have read the CONTRIBUTING guide.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

src/vorta/views/archive_tab.py

Hofer-Julian

The PR works as expected.
I am a bit unsure about the implementation, a bit more discussion is needed here.

src/vorta/views/archive_tab.py

Hofer-Julian

Great job!

real-yfprojects

Until now the buttons where visually grouped into single and multi archive options. This PR dismisses this grouping logic. However I am starting to think that grouping the buttons like that might not be so useful afterall. What do you think?

src/vorta/views/archive_tab.py

real-yfprojects · 2023-06-14T17:16:10Z

src/vorta/views/archive_tab.py

+                    job.updated.connect(self._set_status)
+                    job.result.connect(self.info_result)
+                    self._toggle_all_buttons(False)
+                    self.app.jobs_manager.add_job(job)

    def info_result(self, result):
        self._toggle_all_buttons(True)


This will enable the buttons again although the other info jobs are still running.

So should I connect just the last job's result to this function?

Well, theoretically you don't know which job finishes first. Did you try to run the current code? I would think that only one borg jobs succeeds because of the repo lock.

Yeah I tried running it (I attached a GIF above).

Nevermind, I didn't attach the GIF. Here,

They do blink, but all jobs are succeeding right?

m3nu · 2023-06-14T21:04:24Z

The enabled/disabled state of buttons seems to go on and off. Or is that just the GIF?

And didn't we want to rename "Refresh" to something else? I recall a discussion here or elsewhere.

diivi · 2023-06-14T21:22:08Z

And didn't we want to rename "Refresh" to something else? I recall a discussion here or elsewhere.

Ah yes, done in the next commit.

The enabled/disabled state of buttons seems to go on and off.

Yes, I am confused how to disable them until all jobs are completed though (so they don't get enabled after every single job), looking around.

m3nu · 2023-06-14T21:31:00Z

Yes, I am confused how to disable them until all jobs are completed though (so they don't get enabled after every single job), looking around.

Depends on how they are toggled right now. If it's from the BorgJob directly, there isn't much to be done without larger changes.

diivi · 2023-06-14T21:55:25Z

If it's from the BorgJob directly, there isn't much to be done without larger changes.

Yeah, it's connected to the BorgJob's result :/

diivi · 2023-06-16T18:55:33Z

Yeah, it's connected to the BorgJob's result :/

What can I do now?

m3nu · 2023-06-23T11:51:34Z

OK for me. An isolated change, which doesn't add much new code.

Comments should be resolved, if any action is needed on them.

diivi · 2023-06-23T18:06:52Z

Comments should be resolved, if any action is needed on them.

I am only stuck with your question here:
#1723 (comment)

But you said

If it's from the BorgJob directly, there isn't much to be done without larger changes.

other issues are solved, so probably ready to merge.

real-yfprojects · 2023-06-24T20:04:23Z

I am only stuck with your question here:
#1723 (comment)

Then we will need to open a follow-up issue for that problem. Or decide to fix the flashing problem first.

jetchirag · 2023-06-24T20:33:44Z

Not sure if I'm missing something but they seem to toggled in archive_tab.py, instead of BorgJob

https://github.com/borgbase/vorta/blob/92608f9eaaa6ac9d4bf28f9fa59edaae6001fe07/src/vorta/views/archive_tab.py#L532C1-L536C48

So if you move self._toggle_all_buttons(False) outside of for loop, it should stop the constant state change. Perhaps also add a parameter to info_result to change state back only conditionally.

Edit: Didn't factor in re-enabling button after last job.
How about maintaining a variable with length of archive_names, passing it onto info_result which would substract one each time and enable buttons back when it's 0?

diivi · 2023-06-24T21:03:36Z

How about maintaining a variable with length of archive_names, passing it onto info_result which would substract one each time and enable buttons back when it's 0?

This seems like a reasonable approach, I'll try this and update the PR, thanks!

…iivi/vorta into feat/refresh-multiple-archives

diivi · 2023-06-25T14:45:27Z

It ended up being a little more complex than that 😅, but it works the way we want it to now:

Of course I already had the sizes recalculated so it looks like there's less user feedback, but for no info it looks good.

m3nu · 2023-07-04T18:10:52Z

Minor issue, maybe due to rebasing just now: The "Start" button stays disabled after it's done.

Minor issue after rebasing?

…iivi/vorta into feat/refresh-multiple-archives

diivi · 2023-07-05T03:23:21Z

Updated so that the start backup button gets enabled again after refreshing all archives.

m3nu · 2023-07-05T10:27:00Z

Works as expected locally. Merging. Thanks for your patience on polishing this. 👏

feat: refresh multiple archives

24b5197

diivi commented Jun 1, 2023

View reviewed changes

src/vorta/views/archive_tab.py Outdated Show resolved Hide resolved

change conditional for brefresharchive check

888eaea

diivi marked this pull request as ready for review June 2, 2023 13:43

diivi mentioned this pull request Jun 5, 2023

feat: remove compact button for older borg versions < 1.2 #1727

Merged

9 tasks

Hofer-Julian reviewed Jun 9, 2023

View reviewed changes

src/vorta/views/archive_tab.py Outdated Show resolved Hide resolved

src/vorta/views/archive_tab.py Outdated Show resolved Hide resolved

refactor

20a79c3

Hofer-Julian reviewed Jun 9, 2023

View reviewed changes

src/vorta/views/archive_tab.py Outdated Show resolved Hide resolved

single archive action buttons

dd0cbf2

Hofer-Julian previously approved these changes Jun 9, 2023

View reviewed changes

real-yfprojects reviewed Jun 11, 2023

View reviewed changes

src/vorta/views/archive_tab.py Outdated Show resolved Hide resolved

src/vorta/views/archive_tab.py Outdated Show resolved Hide resolved

src/vorta/views/archive_tab.py Outdated Show resolved Hide resolved

group multi archive buttons

d1bd6f7

diivi dismissed Hofer-Julian’s stale review via d1bd6f7 June 12, 2023 19:38

real-yfprojects reviewed Jun 14, 2023

View reviewed changes

src/vorta/views/archive_tab.py Show resolved Hide resolved

real-yfprojects reviewed Jun 14, 2023

View reviewed changes

refactor and rename refresh to recalc

cecc66f

This was referenced Jun 18, 2023

feat: add a trigger column to the archive table #1732

Merged

feat: inline edit for archive renaming #1734

Merged

m3nu self-assigned this Jun 23, 2023

diivi added 2 commits June 23, 2023 23:42

set status on rename fail

133216f

Merge branch 'master' into feat/refresh-multiple-archives

0997825

diivi added 2 commits June 25, 2023 20:12

enable buttons only when all archives are refreshed

f2bd1a0

Merge branch 'feat/refresh-multiple-archives' of https://github.com/d…

7dac1ba

…iivi/vorta into feat/refresh-multiple-archives

Merge branch 'master' into feat/refresh-multiple-archives

5884a88

m3nu previously approved these changes Jun 28, 2023

View reviewed changes

Merge branch 'master' into feat/refresh-multiple-archives

33b241f

diivi added 2 commits July 5, 2023 08:52

re enable create button when refreshing is done

19f666e

Merge branch 'feat/refresh-multiple-archives' of https://github.com/d…

69f8243

…iivi/vorta into feat/refresh-multiple-archives

fix tests

8c3179e

m3nu approved these changes Jul 5, 2023

View reviewed changes

m3nu merged commit 157ac37 into borgbase:master Jul 5, 2023

diivi mentioned this pull request Feb 19, 2024

Refresh archive details of multiple archives selected. #1367

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: refresh multiple archives #1723

feat: refresh multiple archives #1723

diivi commented Jun 1, 2023 •

edited

Loading

Hofer-Julian left a comment

Hofer-Julian left a comment

real-yfprojects left a comment

real-yfprojects Jun 14, 2023

diivi Jun 14, 2023

real-yfprojects Jun 14, 2023

diivi Jun 14, 2023

diivi Jun 14, 2023 •

edited

Loading

m3nu commented Jun 14, 2023

diivi commented Jun 14, 2023

m3nu commented Jun 14, 2023

diivi commented Jun 14, 2023

diivi commented Jun 16, 2023

m3nu commented Jun 23, 2023

diivi commented Jun 23, 2023 •

edited

Loading

real-yfprojects commented Jun 24, 2023

jetchirag commented Jun 24, 2023 •

edited

Loading

diivi commented Jun 24, 2023 •

edited

Loading

diivi commented Jun 25, 2023

m3nu commented Jul 4, 2023

diivi commented Jul 5, 2023

m3nu commented Jul 5, 2023

feat: refresh multiple archives #1723

feat: refresh multiple archives #1723

Conversation

diivi commented Jun 1, 2023 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Hofer-Julian left a comment

Choose a reason for hiding this comment

Hofer-Julian left a comment

Choose a reason for hiding this comment

real-yfprojects left a comment

Choose a reason for hiding this comment

real-yfprojects Jun 14, 2023

Choose a reason for hiding this comment

diivi Jun 14, 2023

Choose a reason for hiding this comment

real-yfprojects Jun 14, 2023

Choose a reason for hiding this comment

diivi Jun 14, 2023

Choose a reason for hiding this comment

diivi Jun 14, 2023 • edited Loading

Choose a reason for hiding this comment

m3nu commented Jun 14, 2023

diivi commented Jun 14, 2023

m3nu commented Jun 14, 2023

diivi commented Jun 14, 2023

diivi commented Jun 16, 2023

m3nu commented Jun 23, 2023

diivi commented Jun 23, 2023 • edited Loading

real-yfprojects commented Jun 24, 2023

jetchirag commented Jun 24, 2023 • edited Loading

diivi commented Jun 24, 2023 • edited Loading

diivi commented Jun 25, 2023

m3nu commented Jul 4, 2023

diivi commented Jul 5, 2023

m3nu commented Jul 5, 2023

diivi commented Jun 1, 2023 •

edited

Loading

diivi Jun 14, 2023 •

edited

Loading

diivi commented Jun 23, 2023 •

edited

Loading

jetchirag commented Jun 24, 2023 •

edited

Loading

diivi commented Jun 24, 2023 •

edited

Loading