Skip to content

Conversation

@liveHarshit
Copy link
Member

Fixes #2142

@liveHarshit
Copy link
Member Author

@iamareebjamal Koin module is not refreshing, event after opening the app again.

@iamareebjamal
Copy link
Member

See how it is done in Orga App

@liveHarshit
Copy link
Member Author

See how it is done in Orga App

In orga app, URL is directly set in Interceptor but here we are using koin module for retrofit build. I have tried to set URL in request builder, but not working.

@liveHarshit
Copy link
Member Author

val preference = Preference()
    var url = preference.getString(API_URL)

    override fun intercept(chain: Interceptor.Chain): Response {
        val authorization = authHolder.getAuthorization()
        val request = chain.request().newBuilder()
        val httpUrl = url
        if (httpUrl != null) {
            request.url(httpUrl)
        }
        return if (authorization != null) {
            request.header("Authorization", authorization)
            chain.proceed(request.build())
        } else
            chain.proceed(request.build())
    }

@iamareebjamal
Copy link
Member

We are using dagger there, so no difference. You have to implement it similarly, using Interceptor

@liveHarshit liveHarshit changed the title [WIP] feat: Add server configuration field for fDroid feat: Add server configuration field for fDroid Jul 19, 2019
@liveHarshit
Copy link
Member Author

Updated. Working 👍

ShridharGoel
ShridharGoel previously approved these changes Jul 19, 2019
override fun intercept(chain: Interceptor.Chain): Response {
var original = chain.request()
val httpUrl = preference.getString(API_URL)?.toHttpUrlOrNull()
if (httpUrl != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has a flaw which is in Orga App as well. This will replace every URL with API URL. We want only the API URL to be replaced

Copy link
Member Author

@liveHarshit liveHarshit Jul 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this same solution: https://gist.github.com/swankjesse/8571a8207a5815cca1fb#file-hostselectioninterceptor-java-L19
One guy commenting that it will change all URLs: https://gist.github.com/swankjesse/8571a8207a5815cca1fb#gistcomment-2160202 and provide following solution for setting multiple API URLs using RetrofitUrlManager.getInstance().putDomain() : https://github.com/JessYanCoding/RetrofitUrlManager/blob/master/app/src/main/java/me/jessyan/retrofiturlmanager/demo/MainActivity.java
Can you please check it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just have to add an if statement, if (BuildConfig.API_URL == incomingUrl)
proceed to change, else skip

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How? BuildConfig.API_URL is always the default URL, and incomingUrl will change according to user preference. If they are equal then no need to change. I think it should reverse.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BuildConfig.API_URL will be the base URL. So incoming URL will be BuildConfig.API_URL if it is API call. In which case, we'll replace it with the URL in preference. If it is not BuildConfig.API_URL, then it may be for different domain, like google.com, in which case, we'll not replace it because it is not our API domain. Clear?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BuildConfig.API_URL will be the base URL. So incoming URL will be BuildConfig.API_URL if it is API call. In which case, we'll replace it with the URL in preference. If it is not BuildConfig.API_URL, then it may be for different domain, like google.com, in which case, we'll not replace it because it is not our API domain. Clear?

Yes ✌️

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@iamareebjamal iamareebjamal merged commit 762e259 into fossasia:development Jul 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable server configuration field in F-Droid build

3 participants