updateExistingStream: Reduce api calls on changing stream details.

[jainkuniya]

Before inidividual api calls were made for each stream property which
is changed. But on server, this api is capable of handeling multiple
propery at a time. So just call api once with all the properties which
are changed.

Fixes: #3222

[borisyankov]

Do you need to spread the values?
```{ ...newValues }`` vs just newValues

[jainkuniya]

I tried with newValues , then flow is giving error.

Cannot call apiPatch with newValues bound to params because:
 • string [1] is incompatible with boolean [2] in property description.
 • string [1] is incompatible with number [3] in property description.
 • boolean [4] is incompatible with string [5] in property is_private.
 • boolean [4] is incompatible with number [3] in property is_private.
 • string [6] is incompatible with boolean [2] in property new_name.
 • string [6] is incompatible with number [3] in property new_name.

           src/api/streams/updateStream.js
       [6]  6│   new_name?: string,
       [1]  7│   description?: string,
       [4]  8│   is_private?: boolean,
            9│ };
           10│
           11│ export default (auth: Auth, id: number, newValues: ParamsType): Promise<ApiResponse> =>
           12│   apiPatch(auth, `streams/${id}`, res => res, newValues);
           13│

           src/utils/url.js
 [5][2][3] 16│ export type UrlParams = { [string]: string | boolean | number };



Found 6 errors

In toggleMobilePushSettings also we are creating new object.

export default async ({
  auth,
  opp,
  value,
}: {
  auth: Auth,
  opp: string,
  value: boolean,
}): Promise<ApiResponse> =>
  apiPatch(auth, 'settings/notifications', res => res, {
    ...getRequestBody(opp, value),
  });

So I thought of doing same 😅

[borisyankov]

Is this an exhaustive list?

[jainkuniya]

yup! stream have limited properties.

[borisyankov]

ParamsType does not sound like concrete or a good name.
First, we don't name types *Type :)
Also, Param? It is a parameter/argument to a function but you can do better.

[jainkuniya]

any name you want to suggest? :)
ApiBody, ApiParameters??

[borisyankov]

This looks like a candidate for a utility function.
The chances are that we will add more fields and we will need this logic in other places.
What if you do create a currentValues object with the values needed.
Then we can diff it against the current version (via a utility function)

[jainkuniya]

Then we need to do some refactors, like

• currently we have name key (for stream name) in initialValues and newValues
  But at the time of sending parameters to API, we want key new_name, so again a if/else will be required.
  Suggestions?

[borisyankov]

I would suggest you do an object diffing function and then pass the resulting object (if it is not empty!) to this new function:
https://github.com/zulip/zulip-mobile/blob/master/src/api/apiFetch.js#L14

[jainkuniya]

(#3140 is blocked on this)

[jainkuniya]

@borisyankov Updated 👍

[gnprice]

We ended up making this change in be12766 / #5341, with a slightly more explicit implementation.