matthieucoisne / yelpexplorer

Android application that demonstrates the differences between using a REST API versus a GraphQL API while following the best practices using Kotlin, Jetpack Compose, Coroutines, Clean Architecture, MVVM, Dependency Injection, Tests...

Hi there 👋

yelpexplorer's Issues

Add Unit Tests

Display the business' open hours in the details screen

Display the hours, that are already retrieved with the BusinessDetailsQuery, in the business details screen

Gradlle not syncing

Hi
i downloaded the repo to see the code and try it on my machine but i got the below error while building the gradle. Any thing i am missing?
Thanks

A problem occurred configuring root project 'YelpExplorer'.
> Could not resolve all artifacts for configuration ':classpath'.
   > Could not resolve org.jetbrains.kotlin:kotlin-android-extensions:1.4.0.
     Required by:
         project : > org.jetbrains.kotlin:kotlin-gradle-plugin:1.4.0
      > No cached version of org.jetbrains.kotlin:kotlin-android-extensions:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-android-extensions:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-android-extensions:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-android-extensions:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-android-extensions:1.4.0 available for offline mode.
   > Could not resolve org.jetbrains.kotlin:kotlin-gradle-plugin-api:1.4.0.
     Required by:
         project : > org.jetbrains.kotlin:kotlin-gradle-plugin:1.4.0
      > No cached version of org.jetbrains.kotlin:kotlin-gradle-plugin-api:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-gradle-plugin-api:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-gradle-plugin-api:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-gradle-plugin-api:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-gradle-plugin-api:1.4.0 available for offline mode.
   > Could not resolve org.jetbrains.kotlin:kotlin-gradle-plugin-model:1.4.0.
     Required by:
         project : > org.jetbrains.kotlin:kotlin-gradle-plugin:1.4.0
      > No cached version of org.jetbrains.kotlin:kotlin-gradle-plugin-model:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-gradle-plugin-model:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-gradle-plugin-model:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-gradle-plugin-model:1.4.0 available for offline mode.
      > No cached version of org.jetbrains.kotlin:kotlin-gradle-plugin-model:1.4.0 available for offline mode.

Possible solution:
 - Disable offline mode and rerun the build

Update README

Migrate from Toothpick to Koin

https://doc.insert-koin.io/
https://medium.com/koin-developers/unboxing-koin-2-1-7f1133ebb790

Remove `closeOnDestroy` for the viewmodel subscopes

KTP.openScopes(ApplicationScope::class)
    .openSubScope(BusinessDetailsViewModel::class) { scope: Scope ->
        scope.installViewModelBinding<BusinessDetailsViewModel>(this)
            .closeOnViewModelCleared(this)
    }
    .closeOnDestroy(this)
    .inject(this)

.closeOnDestroy(this) should not be executed because on rotation, the scope will be closed.
We only want this scope to be closed when the viewmodel gets cleared

Setup ApolloGraphQL and add the queries

https://github.com/apollographql/apollo-android
https://www.yelp.com/developers/graphiql

Fix memory leaks in Fragments

LeakCanary found a leak with our implementation of ViewBindings.

Problem:
Fragments actually have 2 lifecycles, one for the fragment itself, and one for its view.

In the official documentation:
https://developer.android.com/topic/libraries/view-binding#fragments
"Fragments outlive their views. Make sure you clean up any references to the binding class instance in the fragment's onDestroyView() method."

 

Possible fixes:

1- From the official documentation

override fun onDestroyView() {
    binding = null
    super.onDestroyView()
}

2- AutoClearedValue
https://github.com/android/architecture-components-samples/blob/master/GithubBrowserSample/app/src/main/java/com/android/example/github/util/AutoClearedValue.kt

Usage: var xxx by autoCleared<xxxType>()
But that would require to copy/paste AutoClearedValue.kt in each project... not ideal

3- Wait for a fix from Google
https://stackoverflow.com/questions/59503689/could-navigation-arch-component-create-a-false-positive-memory-leak/59504797#59504797
https://www.youtube.com/watch?v=RS1IACnZLy4&feature=youtu.be&t=957

While 3 would be the best, let's fix this with 1 for now.

 

Also, some other improvements can be made:

• Don't keep a reference to the adapter in the BusinessListFragment. The adapter could be accessed via the binding.rvBusinessList and casted to a BusinessListAdapter
• Don't keep a reference to a List<TextView> in BusinessDetailsFragment.

Migrate from Dagger2 to Toothpick3

https://github.com/stephanenicolas/toothpick/

Setup Github Action for PR

fix lint and test commands

Can't execute this command: ./gradlew lintDebug
But this one succeeds: ./gradlew lintGraphqlDebug lintRestDebug

Same for ./gradlew testDebug

> ./gradlew lintDebug

> Configure project :app
WARNING: DSL element 'useProguard' is obsolete.
It will be removed in a future version of the Android Gradle plugin.
Use 'android.enableR8' in gradle.properties to switch between R8 and Proguard.
WARNING: DSL element 'android.viewBinding.enabled' is obsolete and has been replaced with 'android.buildFeatures.viewBinding'.
It will be removed in version 5.0 of the Android Gradle plugin.

> Configure project :features:business
WARNING: DSL element 'android.viewBinding.enabled' is obsolete and has been replaced with 'android.buildFeatures.viewBinding'.
It will be removed in version 5.0 of the Android Gradle plugin.

> Configure project :features:settings
WARNING: DSL element 'android.viewBinding.enabled' is obsolete and has been replaced with 'android.buildFeatures.viewBinding'.
It will be removed in version 5.0 of the Android Gradle plugin.

> Configure project :libraries:core
WARNING: DSL element 'android.viewBinding.enabled' is obsolete and has been replaced with 'android.buildFeatures.viewBinding'.
It will be removed in version 5.0 of the Android Gradle plugin.

FAILURE: Build failed with an exception.

* What went wrong:
Task 'lintDebug' not found in root project 'YelpExplorer'.

* Try:
Run gradlew tasks to get a list of available tasks. Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 1s

Add UI Tests

Update the launcher icon

Change the default icon to a new one with the red color from yelp

Improve toothpick implementation

Apply suggestions from Stephane Nicolas

Modularize + Clean Architecture

Add visual feedback for when the app is loading data

Show at least a spinner in both screens.
Displaying an animation with Lottie would be even better.

build.gradle cleanup

ideas: https://github.com/cortinico/kotlin-android-template

Display user reviews in the details screen

Add a REST implementation to compare with GraphQL

I wanted to see how much work it would take and how many layers would be affected from moving from GraphQL to REST. I thought that with a clean architecture, there would only be modifications in the data package, but it turns out that with REST, I have to combine several API calls to reproduce what I have with GraphQL, so use cases should also be updated.

To better demonstrate this, use 2 versions to better separate the 2 implementations.

Use Coroutines+Flow instead of ApolloCall.Callback

https://www.apollographql.com/docs/android/advanced/coroutines/
https://codelabs.developers.google.com/codelabs/advanced-kotlin-coroutines/index.html#1

Crash when clicking multiple times on a business in the business list

Steps to repro:

1. Launch the app and wait for the business list to be loaded
2. Click multiple times with multiple fingers on a business in the list
   -> The app will crash

Stacktrace:

2020-02-15 15:10:01.642 14060-14060/com.yelpexplorer E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.yelpexplorer, PID: 14060
    java.lang.IllegalArgumentException: navigation destination com.yelpexplorer:id/action_businessListFragment_to_businessDetailsFragment is unknown to this NavController
        at androidx.navigation.NavController.navigate(NavController.java:892)
        at androidx.navigation.NavController.navigate(NavController.java:833)
        at androidx.navigation.NavController.navigate(NavController.java:819)
        at com.yelpexplorer.features.business.presentation.businesslist.BusinessListFragment.navigateToDetails(BusinessListFragment.kt:103)
        at com.yelpexplorer.features.business.presentation.businesslist.BusinessListFragment.access$navigateToDetails(BusinessListFragment.kt:28)