-
Notifications
You must be signed in to change notification settings - Fork 554
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
Fix #659: Lowfi admin control part 3 logout appversion #755
Fix #659: Lowfi admin control part 3 logout appversion #755
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.
Looks good. But there are nit changes.
Also, click on back
button in toolbar in AppVersionActivity
is not working, fix that and also add test case of that.
app/src/sharedTest/java/org/oppia/app/administratorcontrols/AppVersionActivityTest.kt
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/administratorcontrols/AppVersionActivityTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/administratorcontrols/AppVersionActivityTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/administratorcontrols/AppVersionActivityTest.kt
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/administratorcontrols/AppVersionActivityTest.kt
Show resolved
Hide resolved
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, nit changes suggested.
...rcontrols/administratorcontrolsitemviewmodel/AdministratorControlsAccountActionsViewModel.kt
Outdated
Show resolved
Hide resolved
...src/sharedTest/java/org/oppia/app/administratorcontrols/AdministratorControlsActivityTest.kt
Outdated
Show resolved
Hide resolved
...src/sharedTest/java/org/oppia/app/administratorcontrols/AdministratorControlsActivityTest.kt
Outdated
Show resolved
Hide resolved
...src/sharedTest/java/org/oppia/app/administratorcontrols/AdministratorControlsActivityTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/administratorcontrols/AppVersionActivityTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/administratorcontrols/AppVersionActivityTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/administratorcontrols/AppVersionActivityTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/administratorcontrols/AppVersionActivityTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/administratorcontrols/AppVersionActivityTest.kt
Outdated
Show resolved
Hide resolved
@rt4914 This is the AppVersion Screen. I am have not created any hifi branch for this screen because Its done according to mock already. If u find anything then please tell me. |
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.
Nit final changes.
...src/sharedTest/java/org/oppia/app/administratorcontrols/AdministratorControlsActivityTest.kt
Outdated
Show resolved
Hide resolved
...src/sharedTest/java/org/oppia/app/administratorcontrols/AdministratorControlsActivityTest.kt
Outdated
Show resolved
Hide resolved
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.
Just merge your branch with latest develop and we can merge it after that.
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.
Nit.
Explanation
Fix #659 Lowfi admin control part 3: AppVersion and Logout
Checklist