Take-Home Assignment Submission

DOCUMENTATION.md

@@ -0,0 +1,149 @@
+# Documentation
+
+This README file includes some details about the way I approached the assignment. It's my hope that
+it clarifies my thought process while solving the problem at hand and communicates some of the
+decisions and assumptions I made along the way.
+
+## Approach
+
+Below is a high-level step-by-step plan for approaching the assignment:
+
+### Step 0 - Get started
+
+- [x] Create this document
+
+### Step 1 - Preparation
+
+- [x] Fork of the repository
+- [x] Run the code and see the features in action
+- [x] Get familiar with the code
+- [x] Read the tasks thoroughly and ask for clarifications if needed
+- [x] Make a plan for how you want to approach every task - include an estimate for time-boxing.
+  Include testing and documentation in the plan and the estimate
+
+### Step 2 - Complete the main tasks
+
+- [x] Complete the main tasks one by one based on the priority. Write tests and document decisions
+  and assumptions as you go.
+    - [x] Task 1 - Navigation Framework Refactoring
+    - [x] Task 2 - Implement a Fake GithubService and ViewModel unit tests
+    - [x] Task 3 - Add a feature: User Details
+- [x] Ensure all the requirements in the description are fulfilled before moving on to the bonus
+  points
+
+### Step 3 - Tackle the bonus tasks
+
+- [x] If the time permits tackle the bonus tasks. Otherwise write some documentation explaining how
+  you would have approached them
+
+### Step 4 - Finalise & submit
+
+- [x] Read the description again and make sure you have completed all the tasks
+- [x] Create a PR
+
+## Plan
+
+### Task 1 - Navigation Framework Refactoring
+
+Before starting any major refactoring it's important to ensure we have tests covering the
+functionalities the refactoring can impact.
+So as a first step we will write some UI tests covering the navigation logic.
+
+Once we have the tests in place, we will create fragments for every screen in the app. The code in
+the fragments will not be very different from
+what we currently have in the controllers. The only main difference would be the navigation logic to
+other screens.
+
+With that in place, we can go ahead and create our navigation graph. Some updates might be required
+to the current API of the features, but will try to minimize
+breaking changes so that we can do not break existing module while doing the migration.
+
+Next we will replace the conductor navigation logic with our Navigation component set up and make
+sure the tests we created still pass.
+
+Finally we will delete all the code related to conductor navigation.
+
+Estimation: 3 hours
+
+### Task 2 - Implement a Fake GithubService and ViewModel unit tests
+
+To start this task we create a `FakeGithubService` that conforms to our `GithubService` interface.
+This will provide us with fake data we need to unit test the viewmodel we pick for testing.
+We will need to expose additional properties or functions to allow us control the fake data we
+expect in different tests.
+
+The unit tests should cover the happy path and the edge cases.
+
+Estimation: 1 hour
+
+### Task 3 - Add a feature: User Details
+
+This includes the following changes
+
+- Tweaks to `GithubService` to fetch user details and map it to a domain model
+- A ViewModel that fetches the user details from `GithubService` and maps it to a UI model
+- A new fragment containing the actual UI - might do this in Compose to showcase my experience with
+  it
+- Adding the new fragment as a new navigation destination
+- Adding logic to navigate to the new fragment
+
+Estimation: 2 hours
+
+## Bonus tasks
+
+Below are the following bonus tasks I managed to complete:
+
+- Address usability issues:
+    - Hiding the keyboard when the user presses the search button.
+    - Displaying a "No results found" message when the search query yields no results.
+    - Fix the navigation backstack issue when navigating from contributor to repository to contributor.
+- Code cleanup:
+    - Removing unused resources and files.
+    - Organizing and rearranging imports.
+    - Future improvement: Adding a static code analyzer like Detekt to detect code smells and
+      automatically format code.
+- Creating a second app (`pink_browser`) with a different theme.
+
+And this is how I would approach the remaining tasks:
+
+- Creating another app with a subset of features:
+    - We can build a graph with a subset of destinations. Providing features with more context about
+      available features so they can adjust themselves accordingly can be achieved by passing
+      arguments to features, but there might be a more elegant solution.
+- Caching GitHub API data:
+    - This can be accomplished by introducing a repository layer for our features and a local data
+      source that persists the results received from the network. The repository fetches the results
+      from the network and caches them in the local data source the first time they are received.
+      Subsequent calls to the repository skip fetching the data from the API and return the cached
+      data from the local data source.
+
+## Decision log
+
+- To wait for data load in the UI tests I considered two options: Idling resource which is an API
+  Espresso provides to wait for asynchronous operations and replacing the Github service DI binding
+  with a fake one.
+  Decided to go with the latter because it does not require adding test related code in the
+  production source code and provides more control over the test (no risk for flaky tests).
+- We need a fragment per screen to build our navigation graph. To scope the ViewModels to fragments
+  I decided to use `HiltViewModel` that takes care of injections without us needing to declare extra
+  Dagger components.
+  For that I created a new temporary ViewModel for every screen. This extra ViewModel is not ideal
+  but helps us avoid breaking the app while we add the navigation bits and pieces. This will later
+  replace the ViewModel once we remove all the conductor specific code.
+- I decided to move nav destination declarations to individual feature modules. This keeps the app
+  module cleaner and enables us to expand destinations into nested graphs if needed in the future
+  without touching the app(s).
+- While writing unit tests I considered including a assertion library but realized it might be an
+  overkill for this task.
+- While covering `RepositorySearchViewModel` with unit tests I tweaked the implementation to cover
+  an edge case (search called with an empty string).
+- Introduced a new `UserDetails` domain model that includes more details about a user. This will be
+  mapped to a UI model in our new user details screen.
+- Decided to build the UI for user details screen in Compose to showcase my experience with Compose UI.
+- Considered passing the data we have already fetched on the user (avatar, name and login) to the
+  user details. That way user sees something some details about the user while we fetch the rest.
+  But eventually decided to load all the details and display them in one go. There is a loading
+  state (simple spinner) the user sees in the user details screen while the data loads.
+- Decided to fix the infinite forward navigation issue by clearing the backstack. An alternative I
+  considered was disabling item click on the list of repositories. But decided not to do that as
+  that would have changed a potentially useful functionality.

README.md

@@ -1,4 +1,7 @@
-This repository contains android code for a simple app that can search through github repositories and users.
+This repository contains android code for a simple app that can search through github repositories
+and users.
 
-It is intended to serve as a basis for android tech assignments. As such it is not intended to be "production complete" in any sense. 
-But to serve as a foundation for exploring ways to extend, improve and refactor functionality - both in terms of user and developer experience.
+It is intended to serve as a basis for android tech assignments. As such it is not intended to be "
+production complete" in any sense.
+But to serve as a foundation for exploring ways to extend, improve and refactor functionality - both
+in terms of user and developer experience.

apps/browser/build.gradle

@@ -23,7 +23,7 @@ android {
         versionCode 1
         versionName "1.0"
 
-        testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
+        testInstrumentationRunner "com.kuba.example.githubbrowser.CustomTestRunner"
     }
 
     buildTypes {
@@ -39,14 +39,16 @@ android {
     kotlinOptions {
         jvmTarget = '1.8'
     }
+    testOptions {
+        animationsDisabled = true
+    }
 }
 
 dependencies {
     implementation project(":features:projects:impl")
     implementation project(":features:users:impl")
     implementation project(":shared:service:impl")
-    implementation project(":shared:navigation:impl")
-    implementation project(":shared:dagger-conductor")
+    implementation project(":shared:navigation:api")
 
     // Core
     implementation libs.androidx.activity
@@ -62,7 +64,8 @@ dependencies {
     implementation libs.dagger.android
 
     // Navigation
-    implementation libs.conductor
+    implementation libs.navigation.fragment.ktx
+    implementation libs.navigation.ui.ktx
 
     // Features
     implementation project(":features:projects:impl")
@@ -71,4 +74,5 @@ dependencies {
     testImplementation libs.junit
     androidTestImplementation libs.androidx.junit
     androidTestImplementation libs.androidx.espresso.core
-}
+    androidTestImplementation libs.dagger.hilt.testing
+}

apps/browser/src/androidTest/java/com/kuba/example/githubbrowser/CustomTestRunner.kt

@@ -0,0 +1,14 @@
+package com.kuba.example.githubbrowser
+
+import android.app.Application
+import android.content.Context
+import androidx.test.runner.AndroidJUnitRunner
+import dagger.hilt.android.testing.HiltTestApplication
+
+// A custom runner to set up the instrumented application class for tests.
+class CustomTestRunner : AndroidJUnitRunner() {
+
+    override fun newApplication(cl: ClassLoader?, name: String?, context: Context?): Application {
+        return super.newApplication(cl, HiltTestApplication::class.java.name, context)
+    }
+}

apps/browser/src/androidTest/java/com/kuba/example/githubbrowser/FakeGithubService.kt

@@ -0,0 +1,49 @@
+package com.kuba.example.githubbrowser
+
+import com.kuba.example.service.api.GithubService
+import com.kuba.example.service.api.Repository
+import com.kuba.example.service.api.ServiceResult
+import com.kuba.example.service.api.User
+import com.kuba.example.service.api.UserDetails
+import javax.inject.Inject
+
+class FakeGithubService @Inject constructor() : GithubService {
+    companion object {
+        val fakeUser = User(
+            login = "dr-dre",
+            name = "Dr. Dre",
+            avatarUrl = null
+        )
+
+        val fakeRepository = Repository(
+            id = 1,
+            name = "Dres-adventures",
+            description = "Adventures of Dr. Dre",
+            ownerLogin = "Dr-dre",
+            stars = 1000
+        )
+    }
+
+    override suspend fun searchRepos(query: String): ServiceResult<List<Repository>> {
+        val repositories = listOf(fakeRepository)
+        return ServiceResult.Success(repositories)
+    }
+
+    override suspend fun getContributors(
+        login: String,
+        repositoryName: String
+    ): ServiceResult<List<User>> {
+        val contributors = listOf(fakeUser)
+        return ServiceResult.Success(contributors)
+    }
+
+    override suspend fun getUserDetails(login: String): ServiceResult<UserDetails> {
+        TODO("Not yet implemented")
+    }
+
+    override suspend fun getUserRepos(login: String): ServiceResult<List<Repository>> {
+        val repositories = listOf(fakeRepository)
+        return ServiceResult.Success(repositories)
+    }
+
+}

apps/browser/src/androidTest/java/com/kuba/example/githubbrowser/NavigationTest.kt

@@ -0,0 +1,86 @@
+package com.kuba.example.githubbrowser
+
+import androidx.test.espresso.Espresso.onView
+import androidx.test.espresso.action.ViewActions.click
+import androidx.test.espresso.action.ViewActions.typeText
+import androidx.test.espresso.assertion.ViewAssertions.matches
+import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
+import androidx.test.espresso.matcher.ViewMatchers.withId
+import androidx.test.espresso.matcher.ViewMatchers.withText
+import androidx.test.ext.junit.rules.ActivityScenarioRule
+import androidx.test.filters.LargeTest
+import com.kuba.example.service.api.GithubService
+import com.kuba.example.service.impl.di.ServiceModule
+import dagger.Binds
+import dagger.Module
+import dagger.hilt.InstallIn
+import dagger.hilt.android.testing.HiltAndroidRule
+import dagger.hilt.android.testing.HiltAndroidTest
+import dagger.hilt.android.testing.UninstallModules
+import dagger.hilt.components.SingletonComponent
+import org.hamcrest.CoreMatchers.allOf
+import org.junit.Rule
+import org.junit.Test
+
+@LargeTest
+@HiltAndroidTest
+@UninstallModules(ServiceModule::class)
+class NavigationTest {
+
+    @get:Rule
+    val activityRule = ActivityScenarioRule(MainActivity::class.java)
+
+    @get:Rule
+    var hiltRule = HiltAndroidRule(this)
+
+    @Test
+    fun successfulNavigationFromSearchToContributors() {
+        val fakeRepoName = FakeGithubService.fakeRepository.name
+
+        // Fill in a query and click search
+        onView(withId(com.kuba.example.projects.impl.R.id.edit_query))
+            .perform(typeText(fakeRepoName))
+        onView(withId(com.kuba.example.projects.impl.R.id.btn_search))
+            .perform(click())
+
+        // Perform a click on the repository card
+        onView(allOf(withText(fakeRepoName), withId(com.kuba.example.projects.impl.R.id.lbl_name)))
+            .perform(click())
+
+        // Verify that we have navigated to contributors screen
+        onView(withId(com.kuba.example.projects.impl.R.id.contributors_screen))
+            .check(matches(isDisplayed()))
+    }
+
+    @Test
+    fun successfulNavigationFromContributorsToUserRepository() {
+        val fakeRepoName = FakeGithubService.fakeRepository.name
+
+        // Fill in a query and click search
+        onView(withId(com.kuba.example.projects.impl.R.id.edit_query))
+            .perform(typeText(fakeRepoName))
+        onView(withId(com.kuba.example.projects.impl.R.id.btn_search))
+            .perform(click())
+
+        // Perform a click on the repository card
+        onView(allOf(withText(fakeRepoName), withId(com.kuba.example.projects.impl.R.id.lbl_name)))
+            .perform(click())
+
+        // Perform a click on the contributor
+        val fakeLoginName = FakeGithubService.fakeUser.name
+        onView(withText(fakeLoginName))
+            .perform(click())
+
+        // Verify we that have navigated to user repository screen
+        onView(withId(com.kuba.example.users.impl.R.id.user_repositories_screen))
+            .check(matches(isDisplayed()))
+    }
+
+    @Module
+    @InstallIn(SingletonComponent::class)
+    interface TestModule {
+
+        @Binds
+        fun bindGithubService(githubService: FakeGithubService): GithubService
+    }
+}