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

Fixed: 3.9.0 Welcome screen does not run, and Kiwix does not prompt for permissions. #3618

Merged
merged 2 commits into from
Dec 23, 2023

Conversation

MohitMaliFtechiz
Copy link
Collaborator

@MohitMaliFtechiz MohitMaliFtechiz commented Dec 18, 2023

Fixes #3611

  • This PR addresses the scenario where, for any reason, shared preferences and the database are retained on the device, and the list of ZIM files is visible in the library. However, the MANAGE_EXTERNAL_PERMISSION is not found, and when a user attempts to open a file, the application lacks the necessary permission. To improve this scenario, we now prompt the user for permission if it is not available when opening ZIM files from the library.
  • Additionally, fixed the behavior of the swipe refresh layout if the user clicks on the "NO" button in the permission dialog, since it was showing loading progress and did nothing.
  • Removed the lint suppression for "NewApi" which is unnecessary here.

Issue

PermissionAndLoadingIconIssue.mp4

Fix

PermissionAndLoadingIconFix.mp4

Copy link

codecov bot commented Dec 18, 2023

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (fa3e2d1) 48.75% compared to head (0895c54) 48.69%.
Report is 2 commits behind head on main.

❗ Current head 0895c54 differs from pull request most recent head 0e61bdd. Consider uploading reports for the commit 0e61bdd to get more accurate results

Files Patch % Lines
...le/nav/destination/library/LocalLibraryFragment.kt 8.33% 11 Missing ⚠️
...e/nav/destination/library/OnlineLibraryFragment.kt 0.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3618      +/-   ##
============================================
- Coverage     48.75%   48.69%   -0.07%     
+ Complexity     1091     1090       -1     
============================================
  Files           285      285              
  Lines         10556    10563       +7     
  Branches       1413     1415       +2     
============================================
- Hits           5147     5144       -3     
- Misses         4569     4578       +9     
- Partials        840      841       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gouri-panda
Copy link
Collaborator

@MohitMaliFtechiz I can't produce this bug. But I tested/ reviewed the code. @kelson42 what should we do here?

@MohitMaliFtechiz
Copy link
Collaborator Author

@MohitMaliFtechiz I can't produce this bug.

@gouri-panda I was also not able to reproduce the bug, It seems somehow the user's device kept the shared preference and database even user uninstalled the application. But the main bug the user was facing was when he clicked on the ZIM file from the library screen(which is showing due to it being in the database) then he saw the ZIM file not found error due to lack of MANAGE_EXTERNAL_PERMISSION. This PR fixes that problem. So after placing a fix for this, if the user's device kept the sharedpreference and database, it will ask for permission if not granted when opening the ZIM files.

@kelson42
Copy link
Collaborator

kelson42 commented Dec 19, 2023

I'm not in favour about fixing things that we don't understand or can not reproduce.

Don't call anythin "NewAPI", use proper descriptive name.

@kelson42 kelson42 force-pushed the Issue#3611 branch 2 times, most recently from 8325df9 to 0895c54 Compare December 21, 2023 05:35
@kelson42
Copy link
Collaborator

kelson42 commented Dec 21, 2023

Dexription of this pr has not been updated… whatbis the purpose of it?!

@MohitMaliFtechiz
Copy link
Collaborator Author

@kelson42 I have updated the PR description with videos.

@kelson42
Copy link
Collaborator

@MohitMaliFtechiz CI keeps failing…

Copy link
Collaborator

@gouri-panda gouri-panda left a comment

Choose a reason for hiding this comment

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

@MohitMaliFtechiz @kelson42 The code looks good to me! But unfortunately, i can't produce this bug.

@MohitMaliFtechiz
Copy link
Collaborator Author

MohitMaliFtechiz commented Dec 21, 2023

@kelson42 Now CI passed.

MohitMaliFtechiz added 2 commits December 23, 2023 10:25
…or permissions.

* This commit addresses the scenario where, for any reason, shared preferences and the database are retained on the device, and the list of ZIM files is visible in the library. However, the `MANAGE_EXTERNAL_PERMISSION` is not found, and when a user attempts to open a file, the application lacks the necessary permission. To improve this scenario, we now prompt the user for permission if it is not available when opening ZIM files from the library.
* Additionally, enhanced the behavior of the swipe refresh layout if the user clicks on the "NO" button in the permission dialog.
@kelson42 kelson42 merged commit 3c17704 into main Dec 23, 2023
6 of 7 checks passed
@kelson42 kelson42 deleted the Issue#3611 branch December 23, 2023 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.9.0 Welcome screen does not run and kiwix does not prompt for permissions.
3 participants