Sunny-Lau / dirty-room

GNU General Public License v3.0
0 stars 0 forks source link

[Omar]System spaces.avayacloud.com- Crash app if user click on link of Avaya spaces developer Documentation #18

Open sync-by-unito[bot] opened 4 years ago

sync-by-unito[bot] commented 4 years ago

[Kieu]System spaces.avayacloud.com- Crash app if user click on link of Avaya spaces developer Documentation Problem description (be a specific as possible), include step-by-step procedure used to reproduce the issue: Notes:

SV comment for blocking status: This is a blocked issue

Is the issue reproducible: 100%

sync-by-unito[bot] commented 4 years ago

➤ Omar Said commented:

Personal Notes: Seems like the issue is related to the link parsing, when the request is being made, i doesnt return anything (an error). So the messageMime is expecting a value but since there is no value, it doesnt register it and doesnt go through the route of checking if the link contains "/spaces/" or not.

sync-by-unito[bot] commented 4 years ago

➤ Ray Gerami commented:

this issue only happens with spaces development docs link and also is happening on production. So, I'll reduce it to high

sync-by-unito[bot] commented 4 years ago

➤ Ray Gerami commented:

@Omar Said ( https://app.asana.com/0/1176289996827259/calendar ), I see you have assign this back to me again, but I don't see any improvement on the PR. why do we have spaces_domains? is it different per backend? if it does list all the backends then it is wrong.

sync-by-unito[bot] commented 4 years ago

➤ Omar Said commented:

The spaces_domains is unnecessary so I removed it, In the fix, I am only checking if the link contains a similar domain link and if it contains "/spaces/" I recall us talking about adding other extensions (tasks, posts, etc.) for future implementation but you said that my implementation was too complicated so I just left the check for "/spaces/". Please advise.

sync-by-unito[bot] commented 4 years ago

➤ Ray Gerami commented:

This change I think looks better as you considered [location.host]. however, if you can find a way to compare a list of routes would be event better like ['/spaces/', '/tasks/'] where today we will have only this ['/spaces/'] always Consider performance as well.

sync-by-unito[bot] commented 4 years ago

➤ Omar Said commented:

I decided to go with a classic forloop instead of using forEach because forEach doesn't permit you to break out of it halfway, it has to iterate through the whole array. In the forloop, I can break out of the array of routes when a match is found.

sync-by-unito[bot] commented 4 years ago

➤ Omar Said commented:

Implemented reselect version for better performance.