Apps-android-commons: "Failed to send thanks" notification, but thank actually sent successfully

[nicolas-raoul]

Steps to reproduce:

• Compile and run latest master.
• Tap "Review"
• If the picture is good, tap to end a thank
• See notification, it says "Failed to send thanks"

Example:

Even though the thank has actually been sent successfully:

[sivaraam]

I've seen this too.

[gupta-anmol]

Can I try to work on this?

I was able to reproduce this issue for betaDebug. I observed that we get the same failure notification when the thanks is recorded and when it actually fails (I did this by turning off the WiFi just before sending thanks).

[nicolas-raoul]

@6point022 Yes it's yours! Please use "prodDebug" instead. Thanks :-)

[gupta-anmol]

I'll really appreciate some help with this.

This is from the ThankClient.java file

/**
* Handles the Thanking logic
* @param revesionID The revision ID you would like to thank someone for
* @return if thanks was successfully sent to intended recepient, returned as a boolean observable
*/

public Observable<Boolean> thank(long revisionId) {
        try {
            return service.thank(String.valueOf(revisionId), null,
                csrfTokenClient.getTokenBlocking(),
                CommonsApplication.getInstance().getUserAgent())
                .map(mwQueryResponse -> mwQueryResponse.getSuccessVal() == 1);;
        } catch (Throwable throwable) {
            Log.d("Thanked", "Inside throwable");
            return Observable.just(false);
        }
    }

I tried to see what getSuccessVal() will return.

public class MwPostResponse extends MwResponse {
    @Nullable @SuppressWarnings("unused") private String options;
    @SuppressWarnings("unused") private int success;

    public boolean success(@Nullable String result) {
        return "success".equals(result);
    }

    @Nullable public String getOptions() {
        return options;
    }

    public int getSuccessVal() {
        return success;
    }
}

I could be way off here (I am just learning!!), but I think the reason for failure could be that it is actually returning 0, since the default value of success will be 0 and we are never initialising it to anything else. So, in ThankClient.java, Instead of checking the returned value for 1, we should check for 0. Does this seem correct?

[sivaraam]

@6point022 It's a good thing you shared your doubts here :) I'm not so familiar with the code base but I'll share my views here assuming others will correct me if I'm wrong.

... but I think the reason for failure could be that it is actually returning 0, since the default value of success will be 0 and we are never initialising it to anything else

You won't find the code that initializes the value of success anywhere in the app or in the data-client library. It's automatically set when you receive the response for the query (I believe it's done by Retrofit). So, don't worry about the value not being initialized. It would be :)

I think you're in the right track, though. If I were to just go by the example pointed to by the API page, the value of success would be 1 for a successful thank. But the example might be outdated or there might be other values that indicate success. So, I would suggest that you check for what values of success we're incorrectly getting the thank failed notification.

Hope this helps.

[gupta-anmol]

@sivaraam Thanks a lot for the response. ^_^

So, I would suggest that you check for what values of success we're incorrectly getting the thank failed notification.

So, I checked for two cases.

1. When the thanks is sent successfully, getSuccessVal() returns 0, and we're checking for a 1. This is the reason for the incorrect notification.
2. I tried to generate a failure by turning off the phone's WiFi before sending the thanks. In this case, we actually get a throwable and move into the catch block and get the correct notification.

Can you help me understand in what case getSuccessVal() could return a number other than 0, and how to check for them? Also, is it safe to assume that it will only return 0 when there's a success?

[sivaraam]

When the thanks is sent successfully, getSuccessVal() returns 0, and we're checking for a 1. This is the reason for the incorrect notification.

How do you know that the thank succeeded? Did you check the thank log (Special:Log?type=thanks) of the corresponding wiki? I just tried using the API sandbox and could see that the value of success is 1 for a successful thank:

I tried to generate a failure by turning off the phone's WiFi before sending the thanks. In this case, we actually get a throwable and move into the catch block and get the correct notification.

Well, that's what would happen when the app could not connect to the network. But we're actually interested in a different kind of failure here. We have to send a request that _reaches the server_ and the server would respond to us indicating that for some reason the server could not complete the request. It's easier to check that via the API:Sandbox. You can try a thank API request using the following link. Make sure you're logged in before you visit the link.

https://commons.wikimedia.org/wiki/Special:ApiSandbox#action=thank&format=json

I'm not sure for what cases we would be able to see a different value for success though.

Can you help me understand in what case getSuccessVal() could return a number other than 0, and how to check for them?

Ideally it should be present in the API documentation. Unfortunately, the API page doesn't help us here as it doesn't have an exhaustive list of values that success could have.

Also, is it safe to assume that it will only return 0 when there's a success?

It doesn't seem to be returning 0 on success as you could see from the API response above. But if we do get 0 for getSuccessVal() then I think the issue is somewhere else and we would have to figure it out. My knowledge is limited. We'll see if @maskaravivek, @ashishkumar468 or @misaochan could help us here.

I'll just note one thing I observed when checking for this. I was checking if the Wikipedia app (which also uses the data client library albeit a different version, I suppose) was correctly receiving the success value correctly in a MwPostResponse object. I observed that it was indeed getting the value appropriately (MwPostResponse#getSuccessVal() returns 1). Beware that I was checking the response of a different API query [1] [2] and NOT the response of a thank API query.

[gupta-anmol]

Thanks a lot for taking out the time to explain, it's really helpful. 😄

How do you know that the thank succeeded? Did you check the thank log (Special:Log?type=thanks) of the corresponding wiki?

Yes, I checked the thank log and if the thank appeared in it, I considered it a success. If the thank is successful, the value of success returned by getSuccessVal() is actually 0, and not 1.

But if we do get 0 for getSuccessVal() then I think the issue is somewhere else and we would have to figure it out. My knowledge is limited.

Even I am not sure how to proceed from here. Let's see if someone else can help us. :)

[Daykeras]

I'd like to try my hand at figuring out what's going on.

Edit: Actually, there seem to be a lot of bugs involving the JSON being sent to the API. I just noticed that there are other tickets reporting that number of Thanks is _also_ returning 0. I suspect these are related issues.

[misaochan]

Please feel free, @Daykeras !

[Prince-kushwaha]

can i work on this issue

[Daykeras]

Yes

On Thu, Jan 21, 2021, 10:54 AM Prince kushwaha [e-mail]
wrote:

can i work on this issue

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/commons-app/apps-android-commons/issues/3559#issuecomment-764741111,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AD6MJD6SUFOLZT5DSGRTB33S3BE3XANCNFSM4LQ2MSEA
.

[Prince-kushwaha]

ok