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

Save backup import/export location for future import/exports #6319

Merged

Conversation

ATofighi
Copy link
Contributor

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

To save location of import/export, I add a new preference key for import_export_path and save this preference on result of file chooser activity and send it on intent of file chooser on later tries.

So as a user, If a export data on folder foo, when I want to import this data, file chooser show foo as start path.

Fixes the following issue(s)

APK testing

On the website the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.

Due diligence

@ATofighi ATofighi changed the title Save backup import/export location for feature import/exports Save backup import/export location for future import/exports May 17, 2021
Copy link
Collaborator

@XiangRongLin XiangRongLin left a comment

Choose a reason for hiding this comment

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

Did you cover the edge case that i mentioned in the issue?

@ATofighi
Copy link
Contributor Author

Did you cover the edge case that i mentioned in the issue?

Yes, I have tried to cover it in isValidPath function. Is there any other edge cases that I must to cover?

@AudricV AudricV added the feature request Issue is related to a feature in the app label May 21, 2021
@XiangRongLin
Copy link
Collaborator

Is there any other edge cases that I must to cover?

Not that i can think of

@XiangRongLin
Copy link
Collaborator

Can you also remove the conversions between file and path in your tests. Like this XiangRongLin@376e5c1

@ATofighi
Copy link
Contributor Author

ATofighi commented May 21, 2021

Can you also remove the conversions between file and path in your tests. Like this XiangRongLin@376e5c1

Yes, I merge your branch with mine.

Copy link
Collaborator

@XiangRongLin XiangRongLin left a comment

Choose a reason for hiding this comment

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

Codewise looks fine to me now.

@XiangRongLin XiangRongLin merged commit fc7944d into TeamNewPipe:dev May 21, 2021
@ATofighi ATofighi deleted the feat-6039-store-backup-location branch May 21, 2021 19:21
@SameenAhnaf SameenAhnaf mentioned this pull request May 24, 2021
@Stypox
Copy link
Member

Stypox commented Jun 5, 2021

I don't know how to rebase #5415 on top of this... I think that storing the last used path would require some more work in saf.

Stypox added a commit to Stypox/NewPipe that referenced this pull request Jun 6, 2021
TeamNewPipe#6319 and TeamNewPipe#6402 were reverted before adding SAF changes, and have been readded at the end of SAF changes
Stypox added a commit to Stypox/NewPipe that referenced this pull request Jun 6, 2021
TeamNewPipe#6319 and TeamNewPipe#6402 were reverted before adding SAF changes, and have been readded at the end of SAF changes
@Stypox Stypox mentioned this pull request Jun 6, 2021
14 tasks
Stypox added a commit to Stypox/NewPipe that referenced this pull request Jun 8, 2021
TeamNewPipe#6319 and TeamNewPipe#6402 were reverted before adding SAF changes, and have been readded at the end of SAF changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Store backup location
4 participants