Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

searchAnnotations should handle user request errors and maybe pass auth along to request for users #40

Closed
tomwayson opened this issue Jul 12, 2018 · 8 comments
Labels
bug Something isn't working priority - high

Comments

@tomwayson
Copy link
Member

This line sends an unauthenticated request:

const getUserInfo = users.map(name => getUser(name));

I think that will only work for public user accounts. Other user accounts prob return 403.

Also https://github.com/Esri/arcgis-rest-js/pull/243/files is blowing up my https app. Generally it would be a good idea to handle errors for each user request so we still return annos and whatever users are visible. Makes me pine for allSettled.

@ajturner
Copy link
Member

ajturner commented Sep 7, 2018

This is also an issue if the user no longer exists.

For now, I suggest simply

const getUserInfo = users.map(name => getUser(name).catch(error => return null);

and don't push null into the included[] array.

@ajturner ajturner added bug Something isn't working priority - high labels Sep 7, 2018
@tomwayson
Copy link
Member Author

I think this becomes more important w/ the upcoming changes in ArcGIS Online that will prohibit anonymous searches for users.

@jgravois jgravois removed their assignment May 15, 2019
@alexus37
Copy link

alexus37 commented Jun 3, 2019

Another issue in the search function is the getUser function is only called by name (string) and no IGetUserOptions are passed. This crashes if the users from the annotations are from a different portal. The authentication and the portal should be passed on.

@tomwayson
Copy link
Member Author

@alexus37 I'd gladly help you land a PR. :)

@alexus37
Copy link

alexus37 commented Jun 4, 2019

@tomwayson Check out #167

@tomwayson
Copy link
Member Author

@alexus37 sorry for the delay, but we've had a couple of high priority releases/patches of this library that had to go out in the last couple days. I'll look at your PR later today and get it released soon.

@tomwayson
Copy link
Member Author

@alexus37 this was released in v2.2.3

@sandromartis
Copy link
Contributor

Made a new PR #171 to also pass the portal into getUser

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority - high
Projects
None yet
Development

No branches or pull requests

5 participants