Fix #657: Lowfi Admin Controls (Part 1) #665

Luffy18346 · 2020-02-14T17:26:38Z

Explanation

Fix: #657 : Lowfi Admin Controls (Part 1) - Show/hide admin controls option in navigation drawer.

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
The PR follows the style guide.
The PR does not contain any unnecessary auto-generated code from Android Studio.
The PR is made from a branch that's not called "develop".
The PR is made from a branch that is up-to-date with "develop".
The PR's branch is based on "develop" and not on any other branch.
The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

…igation drawer

rt4914

I have taken an overview look at this. PTAL, I will review it in detail later on. Thanks.

app/src/main/AndroidManifest.xml

app/src/main/java/org/oppia/app/drawer/NavigationDrawerFragmentPresenter.kt

app/src/main/res/layout/administrator_controls_fragment.xml

… navigation drawer item

…ntrols button

rt4914

This looks like a very good implementation. Thanks. I have requested some changes. Also, once you open the Administrator Controls on back click it is going to ProfileChooser instead it should go to HOME

app/src/main/AndroidManifest.xml

app/src/main/java/org/oppia/app/administratorcontrols/AdministratorControlsActivity.kt

app/src/main/java/org/oppia/app/drawer/NavigationDrawerFragmentPresenter.kt

app/src/main/java/org/oppia/app/fragment/FragmentComponent.kt

app/src/main/java/org/oppia/app/help/HelpActivity.kt

rt4914 · 2020-02-18T09:12:34Z

app/src/main/java/org/oppia/app/help/HelpActivity.kt

+  companion object {
+    fun createHelpActivityIntent(context: Context, profileId: Int?): Intent {
+      val intent = Intent(context, HelpActivity::class.java)
+      intent.putExtra(KEY_HOME_PROFILE_ID, profileId)


This is not required in HELP

@rt4914 I have done this implementation and also changed both the intent calls in NavigationDrawerFragmentPresenter because otherwise Administrator Controls option won't be visible when we move to Help Activity and then open nav. drawer.

Replied above

app/src/main/res/layout/administrator_controls_fragment.xml

app/src/sharedTest/java/org/oppia/app/testing/NavigationDrawerTestActivityTest.kt

…rols-part-1

rt4914

I think you can change the HelpActivity intent implementation, we don't need profileId in that. Also you have missed my earlier comment: #665 (review)

This looks like a very good implementation. Thanks. I have requested some changes. Also, once you open the Administrator Controls on back click it is going to ProfileChooser instead it should go to HOME and also write test case for checking this.

Along with this non-flaky tests are failing, maybe merge this PR with develop to get latest code.

app/src/main/java/org/oppia/app/drawer/NavigationDrawerFragment.kt

app/src/main/java/org/oppia/app/drawer/NavigationDrawerFragmentPresenter.kt

…s, opens HomeActivity implemented and test-case for that is also written.

…rols-part-1

Luffy18346 · 2020-02-19T20:20:27Z

I think you can change the HelpActivity intent implementation, we don't need profileId in that. Also you have missed my earlier comment: #665 (review)

This looks like a very good implementation. Thanks. I have requested some changes. Also, once you open the Administrator Controls on back click it is going to ProfileChooser instead it should go to HOME and also write test case for checking this.

Along with this non-flaky tests are failing, maybe merge this PR with develop to get latest code.

@rt4914 I have written a testcase, actually extended the code in a test-case which was opening the AdminControlsActivity, there I have put backPressed and checked if recyclerView from homeActivity is visible or not. The test-case is passing successfully.

rt4914

LGTM, just one change regarding the test case.

app/src/sharedTest/java/org/oppia/app/testing/NavigationDrawerTestActivityTest.kt

rt4914 · 2020-02-20T08:08:44Z

app/src/sharedTest/java/org/oppia/app/testing/NavigationDrawerTestActivityTest.kt

+  }
+
+  @Test
+  fun testNavigationDrawerTestActivity_withAdminProfile_openNavigationDrawer_clickAdministratorControls_checkOpensAdministratorControlsActivity_onBackPressed_showsHomeActivity() {


Actually this test case is correct but this test case is not a functionality of NavigationDrawer, instead it is the functionality of AdministratorControlsActivity. So please create a new dedicated test file AdministratorControlsActivityTest and in this file write this test case. Also, note that the function name will be updated accordingly.

I have made a new test file and written same testcase in it. Test is passing successfully. and changed this test-case so that it will now only check for AdministratorActivity opened or not.

…rols-part-1

rt4914

Just one final Nit. Also, always self-review your PR to make your PR merge ready faster. Thanks

rt4914 · 2020-02-21T09:15:32Z

...src/sharedTest/java/org/oppia/app/administratorcontrols/AdministratorControlsActivityTest.kt

+  @Test
+  fun testAdministratorControlsActivity_withAdminProfile_openAdministratorControlsActivityFromNavigationDrawer_onBackPressed_showsHomeActivity() {
+    ActivityScenario.launch<NavigationDrawerTestActivity>(createNavigationDrawerActivityIntent(0)).use {
+      Espresso.onView(withContentDescription(R.string.drawer_open_content_description)).perform(click())


Nit about imports:

onView(withContentDescription(R.string.drawer_open_content_description)).perform(click()) onView(withId(R.id.administrator_controls_linear_layout)).check(matches(isDisplayed())).perform(click()) intended(hasComponent(AdministratorControlsActivity::class.java.name)) intended(hasExtra(AdministratorControlsActivity.getIntentKey(), 0)) onView(isRoot()).perform(pressBack()) onView(withId(R.id.home_recycler_view)).check(matches(isDisplayed()))

…rols-part-1

rt4914

LGTM

Lowfi Admin Controls (Part 1) - Show/hide admin control option in nav…

634ec6b

…igation drawer

Luffy18346 requested a review from rt4914 February 14, 2020 17:26

Luffy18346 assigned rt4914 Feb 14, 2020

rt4914 suggested changes Feb 14, 2020

View reviewed changes

rt4914 assigned Luffy18346 and unassigned rt4914 Feb 14, 2020

Luffy18346 added 4 commits February 15, 2020 23:37

changes in NavigationDrawerFragment to show admin controls from every…

29706da

… navigation drawer item

changes in NavigationDrawerFragment

632f65f

NavigationDrawerFooterViewModel is implemented for show/hide admin co…

3756aa6

…ntrols button

Indentation Issue solved.

077450c

Luffy18346 requested a review from rt4914 February 15, 2020 20:19

Luffy18346 assigned rt4914 and unassigned Luffy18346 Feb 16, 2020

rt4914 suggested changes Feb 18, 2020

View reviewed changes

rt4914 assigned Luffy18346 and unassigned rt4914 Feb 18, 2020

Luffy18346 added 2 commits February 19, 2020 01:13

All the suggessted changes are done except HelpActivity changes.

f8fe765

Merge remote-tracking branch 'upstream/develop' into lowfi-admin-cont…

e7fd65e

…rols-part-1

rt4914 suggested changes Feb 19, 2020

View reviewed changes

Luffy18346 added 3 commits February 20, 2020 01:36

ProfileId parameter changes are done. onBackPressed from AdminControl…

cda7e68

…s, opens HomeActivity implemented and test-case for that is also written.

unused imports removed

2b515e0

Merge remote-tracking branch 'upstream/develop' into lowfi-admin-cont…

e66e904

…rols-part-1

unused imports removed

1c97c2c

Luffy18346 requested a review from rt4914 February 19, 2020 21:19

Luffy18346 assigned rt4914 and unassigned Luffy18346 Feb 19, 2020

rt4914 approved these changes Feb 20, 2020

View reviewed changes

rt4914 assigned Luffy18346 and unassigned rt4914 Feb 20, 2020

Luffy18346 added 2 commits February 21, 2020 00:25

TestCase is re-written in a different file and successfully passed.

fad609d

Changes the test-case implementation in this file.

36c1b05

Luffy18346 requested a review from rt4914 February 20, 2020 19:10

Luffy18346 assigned rt4914 and unassigned Luffy18346 Feb 20, 2020

Merge remote-tracking branch 'upstream/develop' into lowfi-admin-cont…

ad7c07f

…rols-part-1

rt4914 approved these changes Feb 21, 2020

View reviewed changes

rt4914 assigned Luffy18346 and unassigned rt4914 Feb 21, 2020

Luffy18346 added 2 commits February 21, 2020 19:56

Solved nit about imports.

79ca456

Merge remote-tracking branch 'upstream/develop' into lowfi-admin-cont…

5b2e816

…rols-part-1

Luffy18346 requested a review from rt4914 February 21, 2020 14:50

Luffy18346 assigned rt4914 and unassigned Luffy18346 Feb 21, 2020

rt4914 approved these changes Feb 21, 2020

View reviewed changes

rt4914 merged commit 9fcfca7 into oppia:develop Feb 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #657: Lowfi Admin Controls (Part 1) #665

Fix #657: Lowfi Admin Controls (Part 1) #665

Luffy18346 commented Feb 14, 2020

rt4914 left a comment

rt4914 left a comment

rt4914 Feb 18, 2020

Luffy18346 Feb 18, 2020 •

edited

Loading

rt4914 Feb 19, 2020

rt4914 left a comment •

edited

Loading

Luffy18346 commented Feb 19, 2020 •

edited

Loading

rt4914 left a comment

rt4914 Feb 20, 2020

Luffy18346 Feb 20, 2020 •

edited

Loading

rt4914 left a comment

rt4914 Feb 21, 2020

rt4914 left a comment

Fix #657: Lowfi Admin Controls (Part 1) #665

Fix #657: Lowfi Admin Controls (Part 1) #665

Conversation

Luffy18346 commented Feb 14, 2020

Explanation

Checklist

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 Feb 18, 2020

Choose a reason for hiding this comment

Luffy18346 Feb 18, 2020 • edited Loading

Choose a reason for hiding this comment

rt4914 Feb 19, 2020

Choose a reason for hiding this comment

rt4914 left a comment • edited Loading

Choose a reason for hiding this comment

Luffy18346 commented Feb 19, 2020 • edited Loading

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 Feb 20, 2020

Choose a reason for hiding this comment

Luffy18346 Feb 20, 2020 • edited Loading

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 Feb 21, 2020

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

Luffy18346 Feb 18, 2020 •

edited

Loading

rt4914 left a comment •

edited

Loading

Luffy18346 commented Feb 19, 2020 •

edited

Loading

Luffy18346 Feb 20, 2020 •

edited

Loading