-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: delete old videos Directory #316
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test cases are failing.
dashboard/src/main/java/org/openedx/dashboard/presentation/DashboardFragment.kt
Outdated
Show resolved
Hide resolved
dashboard/src/main/java/org/openedx/dashboard/presentation/DashboardFragment.kt
Outdated
Show resolved
Hide resolved
Test Cases are still failing ... |
b9fcf8c
to
29b3fd0
Compare
@volodymyr-chekyrta, @k1rill please spare some time to review this PR. |
if (viewModel.canResetAppDirectory){ | ||
viewModel.resetAppDirectory(this) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we just encapsulate all this inside of the AppViewModel#onCreate
?
In this case, we would remove the extra variable and this logic from the view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, I'll update it accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Test cases are failing due to context, the issue is already addressed in PR-308. I'll update and merge it once PR-308 is merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Let's merge it after the #308
5f65522
to
0d093ea
Compare
Description
This PR implements functionality to delete outdated video files stored in the previous app directory upon the first launch of the updated app version. This ensures that storage is efficiently managed, preventing unnecessary space usage by obsolete files, and maintaining consistency with the new data storage structure.
fix: LEARNER-9950