Remove sending phone number if not present #37
Conversation
|
All these changes can be simplified since empty string are falsy. Alternately you could use Typescript's nullish coalescing functionality. So this: phoneNumber: this.formData.phoneNumber !== '' ? this.formData.phoneNumber : undefinedcan become phoneNumber: this.formData.phoneNumber || undefinedDo you have an easy way to test the API input being generated locally? |
|
No @MasterXen . I am currently using the chrome inspect tool to test the API calls and the params sent. |
|
@MasterXen @tarunkumar2 I’d prefer to have that logic on api layer, so we can actually write tests for it and the views don't need care about it. |
|
Nothing related to this PR. But I have a little suggestion on the UI/UX part for phone number. While testing on sample app, I had issue with not having proper format of my phone number that failed the api request. One little suggestion would be maybe have two separate box on phone number, one for country code and one for actual phone number. Wdyt? |
|
Thank you for the suggestion @timmy-circle . Yeah providing this kind of UI is better since it can avoid the API due to the improper phone number format. I will try to implement this kind of UI option along or provide the placeholder that will help the user determining the format. |
lib/cardsApi.ts
Outdated
| @@ -71,6 +71,22 @@ instance.interceptors.response.use( | |||
| } | |||
| ) | |||
|
|
|||
| function checkNullInMetaData(metaData: MetaData) { | |||
| const nullIfEmpty = (prop: string | undefined) => { | |||
| if (prop === '') { | |||
There was a problem hiding this comment.
Probably a good idea to do prop.trim() === '' to also cover whitespace in a string
lib/cardsApi.ts
Outdated
| sessionId: metaData.sessionId, | ||
| ipAddress: metaData.ipAddress | ||
| } | ||
| return updateMetaObj |
There was a problem hiding this comment.
This replaces the actual metaData object. If there are new properties added, they will be stripped out without noticing.
Best here is to copy the existing obj and only overwrite the properties we want to change.
const updateMetaObj = {
...metaData
email: nullIfEmpty(metaData.email),
phoneNumber: nullIfEmpty(metaData.phoneNumber),
}
lib/cardsApi.ts
Outdated
| @@ -71,6 +71,22 @@ instance.interceptors.response.use( | |||
| } | |||
| ) | |||
|
|
|||
| function checkNullInMetaData(metaData: MetaData) { | |||
| const nullIfEmpty = (prop: string | undefined) => { | |||
There was a problem hiding this comment.
I'd like to have nullIfEmpty reusable across this file. Can we move it out of this function please.
lib/cardsApi.ts
Outdated
| payload.metadata = checkNullInMetaData(payload.metadata) | ||
|
|
||
| return instance.post(url, payload) | ||
| const modifiedPayload = Object.assign({}, payload) |
There was a problem hiding this comment.
I don't think this fixes the issue. Try to use payload:
{
email: 'my-email@xyz.com',
phoneNumber: '+491777777',
sessionId: 'xyz',
ipAddress: '10.0.2.1',
newProp: 'my-prop'
}
newProp will be omitted. That's why I suggested the spread syntax.
There was a problem hiding this comment.
Sorry for not understanding the previous comment. I have updated the code now
lib/cardsApi.ts
Outdated
| return instance.put(url, payload) | ||
| payload.metadata.email = nullIfEmpty(payload.metadata.email) | ||
| payload.metadata.phoneNumber = nullIfEmpty(payload.metadata.phoneNumber) | ||
| return instance.post(url, payload) |
There was a problem hiding this comment.
why did that change to post instead of put?
lib/cardsApi.ts
Outdated
| @@ -133,7 +141,8 @@ function createCard(payload: CreateCardPayload) { | |||
| */ | |||
| function updateCard(cardId: string, payload: UpdateCardPayload) { | |||
| const url = `/v1/cards/${cardId}` | |||
|
|
|||
| payload.metadata.email = nullIfEmpty(payload.metadata.email) | |||
There was a problem hiding this comment.
Can we just have a small check if metadata prop is not undefined. Otherwise i think looks good
The application is sending the phone number as empty value if the phone number is added by the user. Have fixed the application to prevent sending the phone number if the user doesn't enter the phone number.