Added the Retry Installation action to the Add-on Store. by hwf1324 · Pull Request #17104 · nvaccess/nvda

hwf1324 · 2024-09-03T03:46:51Z

Link to issue number:

Closed #17090

Summary of the issue:

There is no Action to retry the installation when downloading/installing an add-on via the Add-on Store fails.

Description of user facing changes

When downloading/installing an add-on fails, you can now retry the installation

Description of development approach

Add Retry Action for failed download/installation status.

Testing strategy:

Disconnect from the network while the add-on is being downloaded.
Connect to the network and use Retry to download Action

Known issues with pull request:

None

Code Review Checklist:

Documentation:
- Change log entry
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
Testing:
- Unit tests
- System (end to end) tests
- Manual testing
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
API is compatible with existing add-ons.
Security precautions taken.

Summary by CodeRabbit

New Features
- Introduced a retry action for selected add-ons in the addon store, allowing users to attempt installation again after a failure.
- Enhanced user documentation to include guidance on retrying add-on installations and clarified exit dialog functionalities.
Documentation
- Updated user documentation to reflect new features and improvements related to add-on installation and troubleshooting.

coderabbitai · 2024-09-03T03:47:20Z

Walkthrough

The changes introduce a new feature in the add-on store that allows users to retry the installation of add-ons if the initial attempt fails. This functionality is implemented through new methods and actions in the relevant code files, enhancing the user interface and experience by providing a straightforward way to address download failures.

Changes

Files	Change Summary
`source/gui/addonStoreGui/controls/actions.py`	Added `BatchAddonActionVM` for retrying selected add-ons with validation checks.
`source/gui/addonStoreGui/viewModels/addonList.py`	Introduced `canUseRetryAction` method to check add-on statuses for retry eligibility.
`source/gui/addonStoreGui/viewModels/store.py`	Updated `_makeActionsList` to include a new retry action for selected add-ons.
`user_docs/en/changes.md`	Documented the new retry feature and clarified updates to user interaction elements.
`user_docs/en/userGuide.md`	Updated instructions for handling installation failures to include retry options.

Assessment against linked issues

Objective	Addressed	Explanation
Add “Retry”/“Redownload” Action when the add-on fails to download (#17090)	✅

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (9)

source/gui/addonStoreGui/controls/actions.py (1)
157-163: LGTM! Consider minor improvement for consistency.

The implementation of the "Retry" action looks good and follows the established pattern. However, for consistency with other actions, consider moving the validCheck lambda to a separate method in the AddonListValidator class, similar to how canUseInstallAction() is used for the "Install" action.

Consider refactoring the validCheck lambda to use a dedicated method:
				validCheck=lambda aVMs: (
					self._storeVM._filteredStatusKey in [_StatusFilterKey.AVAILABLE, _StatusFilterKey.UPDATABLE]
					and AddonListValidator(aVMs).canUseRetryAction()
				),
This change would make the "Retry" action consistent with other actions and improve maintainability.
user_docs/en/changes.md (8)

Line range hint 5-7: Consider expanding on the highlights.

The highlights section provides a good overview, but consider adding a brief sentence or two to explain the significance of each highlight. This could help users quickly understand the impact of major changes.

12-13: Consider clarifying the context for alt+upArrow/alt+downArrow.

While the new feature is clearly described, it might be helpful to specify in which application or context these keyboard shortcuts work.

16-18: Consider providing more context for Microsoft Excel feature.

While the new feature for Microsoft Excel is described, it might be helpful to briefly explain the benefit or use case for this functionality.

Line range hint 22-24: Consider explaining the benefit of assigning input gestures.

While the new feature is clearly described, it might be helpful to briefly explain why users might want to assign input gestures to these settings.

Line range hint 28-30: Consider clarifying the impact of braille display driver settings.

While the new feature is described, it might be helpful to briefly explain the benefit or use case for changing braille display driver settings.

Line range hint 34-35: Consider providing an example for the new command.

While the new command is described, it might be helpful to provide a brief example of when this command might be useful.

Line range hint 52-54: Consider clarifying the impact of the eSpeak NG update.

While the update to eSpeak NG is mentioned, it might be helpful to briefly explain any notable improvements or changes that come with this update.

Line range hint 65-66: Consider providing more context for the braille cursor shape change.

While the change is described, it might be helpful to briefly explain the benefit or use case for this new configuration option.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

hwf1324 · 2024-09-03T04:04:38Z

@seanbudd Thanks for the improved documentation, sorry I can only communicate using translations.

seanbudd · 2024-09-03T04:06:20Z

@hwf1324 - it's okay, we welcome your contributions. thanks for getting some wording up there, it makes it much easier to help.

CyrilleB79 · 2024-09-03T07:16:34Z

@hwf1324, tanks for tackling this issue.

In my opinion, the GUI is not very clear if we add "Retry" action or "Retry selected add-ons" batch action. Something clearer would be "Retry download", etc. However, in this case we would also need "Retry update" and "Retry migrate". This would lead to much mmore code for the same effect.

An other simpler option would just be to allow "Install", "Update", etc even when the previous installation has failed before, i.e. the context menu would not indicate that the action is a retrial. Have you explored this solution?

With this new action

seanbudd · 2024-09-03T07:19:30Z

@CyrilleB79 - that's not possible without either:

resetting the download failed state, which makes users unaware of why their install didn't work
creating an individual failure state for each context, i.e. INSTALL_FAILED, UPDATE_FAILED, REPLACE_FAILED

seanbudd · 2024-09-03T07:20:39Z

I think "retry install" might just be fine, in all those contexts it's still an install.

hwf1324 · 2024-09-03T07:22:18Z

An other simpler option would just be to allow "Install", "Update", etc even when the previous installation has failed before, i.e. the context menu would not indicate that the action is a retrial. Have you explored this solution?

Yes, as I said in this comment, handling them in the base class doesn't make it easy to get the context to use to determine the correct state.

hwf1324 · 2024-09-03T07:24:56Z

I think "retry install" might just be fine, in all those contexts it's still an install.

If you need to change the wording, please just do so.

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

CyrilleB79

OK, if technically we cannot restore the original Install, Update, Migrate actions, I am fine with "Retry".

Here are wording suggestions for this.

XLTechie · 2024-09-03T09:12:40Z

I think "retry install" might just be fine, in all those contexts it's still an install.

But I can see Cyrille's point here: a user might not think of it as an install in some of those other cases. How about "Retry failed operation"?

Qchristensen

Looks good (sorry for my original comment, got distracted on the exit options line when in fact this just cleaned up a typo there :) )

seanbudd · 2024-09-06T01:09:56Z

@hwf1324 - can you resolve conflicts here introduced from #16671

…reinstallation-if-download-fails

seanbudd · 2024-09-06T02:08:12Z

can you try performing a full git clean?

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

stale

AppVeyorBot · 2024-09-06T04:28:52Z

PASS: Translation comments check.
PASS: License check.
PASS: Unit tests.
PASS: Lint check.
FAIL: System tests (tags: installer NVDA). See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/9kpevggp8y7ukr5p/artifacts/output/nvda_snapshot_pr17104-33855,761bcbfe.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 1.1,
INSTALL_END 1.0,
BUILD_START 0.0,
BUILD_END 11.7,
TESTSETUP_START 0.0,
TESTSETUP_END 0.3,
TEST_START 0.0,
TEST_END 18.4,
FINISH_END 0.2

See test results for failed build of commit 761bcbfeb7

Qchristensen

Docs still good

added retry the installation action.

4ca720e

hwf1324 marked this pull request as ready for review September 3, 2024 03:47

hwf1324 requested review from a team as code owners September 3, 2024 03:47

hwf1324 requested review from Qchristensen and SaschaCowley September 3, 2024 03:47

coderabbitai Bot reviewed Sep 3, 2024

View reviewed changes

Comment thread source/gui/addonStoreGui/controls/actions.py

seanbudd and others added 2 commits September 3, 2024 13:57

Update source/gui/addonStoreGui/controls/actions.py

1e848fb

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Pre-commit auto-fix

9d9461c

seanbudd approved these changes Sep 3, 2024

View reviewed changes

Comment thread source/gui/addonStoreGui/viewModels/addonList.py Outdated

Comment thread user_docs/en/changes.md Outdated

Apply suggestions from code review

aba4414

seanbudd reviewed Sep 3, 2024

View reviewed changes

Comment thread source/gui/addonStoreGui/controls/actions.py Outdated

Comment thread source/gui/addonStoreGui/viewModels/store.py Outdated

hwf1324 and others added 2 commits September 3, 2024 15:55

Update source/gui/addonStoreGui/controls/actions.py

b54b9a8

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

Update source/gui/addonStoreGui/viewModels/store.py

6fae58d

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

CyrilleB79 reviewed Sep 3, 2024

View reviewed changes

Comment thread source/gui/addonStoreGui/controls/actions.py

codeofdusk mentioned this pull request Sep 4, 2024

Opt new PRs out of Code Rabbit by default #17112

Closed

This comment was marked as outdated.

Sign in to view

Merge branch 'master' into Allow-reinstallation-if-download-fails

27d170d

Qchristensen previously approved these changes Sep 6, 2024

View reviewed changes

Merge branch 'master' of https://github.com/nvaccess/nvda into Allow-…

b0280b0

…reinstallation-if-download-fails

This comment has been minimized.

Sign in to view

seanbudd reviewed Sep 6, 2024

View reviewed changes

Comment thread user_docs/en/userGuide.md Outdated

Update user_docs/en/userGuide.md

62c1d0e

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

seanbudd reviewed Sep 6, 2024

View reviewed changes

Comment thread user_docs/en/userGuide.md Outdated

Update user_docs/en/userGuide.md

ad5deca

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

seanbudd requested a review from Qchristensen September 6, 2024 02:26

hwf1324 and others added 2 commits September 6, 2024 11:52

Fix not being able to install incompatible add-ons after canceling them

a08e30e

Pre-commit auto-fix

760f41f

This comment has been minimized.

Sign in to view

This comment was marked as off-topic.

Sign in to view

Qchristensen approved these changes Sep 9, 2024

View reviewed changes

Merge branch 'master' into Allow-reinstallation-if-download-fails

def1eac

seanbudd merged commit 76555d6 into nvaccess:master Sep 9, 2024

github-actions Bot added this to the 2025.1 milestone Sep 9, 2024

hwf1324 deleted the Allow-reinstallation-if-download-fails branch September 9, 2024 01:24

Uh oh!

Conversation

hwf1324 commented Sep 3, 2024 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Assessment against linked issues

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hwf1324 commented Sep 3, 2024

Uh oh!

seanbudd commented Sep 3, 2024

Uh oh!

CyrilleB79 commented Sep 3, 2024

Uh oh!

seanbudd commented Sep 3, 2024

Uh oh!

seanbudd commented Sep 3, 2024

Uh oh!

hwf1324 commented Sep 3, 2024

Uh oh!

hwf1324 commented Sep 3, 2024

Uh oh!

Uh oh!

Uh oh!

CyrilleB79 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

XLTechie commented Sep 3, 2024 via email

Uh oh!

This comment was marked as outdated.

Uh oh!

Qchristensen left a comment

Choose a reason for hiding this comment

Uh oh!

seanbudd commented Sep 6, 2024

Uh oh!

This comment has been minimized.

seanbudd commented Sep 6, 2024

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

AppVeyorBot commented Sep 6, 2024

Uh oh!

Qchristensen left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

hwf1324 commented Sep 3, 2024 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 3, 2024 •

edited

Loading