feat(users): add getUserNotifications function #294
Conversation
| @@ -68,3 +68,35 @@ export function getUser( | |||
| // send the request | |||
| return request(url, options); | |||
| } | |||
|
|
|||
| export interface IGetUserNotificationRequestOptions extends IRequestOptions { | |||
There was a problem hiding this comment.
IRequestOptions already includes optional authentication, so no need to extend it with a custom interface.
since you want a UserSession specifically, you could also use IUserRequestOptions
There was a problem hiding this comment.
I was wondering about that but copied it from here. Is that an error or is it not needed there either?
There was a problem hiding this comment.
Oh, now I remember the other thing. I actually tried that but TS complained because the autehentication on IRequestOptions is an IAuthenticationManager, not a UserSession which I think is what I want. I'm a TS noob so advice is welcome.
| @@ -465,3 +465,12 @@ export type esriUnits = | |||
| | "esriSRUnit_Kilometer" | |||
| | "esriSRUnit_NauticalMile" | |||
| | "esriSRUnit_USNauticalMile"; | |||
|
|
|||
| export interface INotification { | |||
There was a problem hiding this comment.
I think this interface could live in @esri/arcgis-rest-users, since it may only be used in that package - or are we putting all interfaces in common-types these days?
There was a problem hiding this comment.
i like where it is, but it'd be fine in this package too.
this is why no one knows where to put interfaces.
| let url; | ||
| let options = { httpMethod: "GET" } as IUserRequestOptions; | ||
|
|
||
| const username = requestOptions.authentication.username; |
There was a problem hiding this comment.
is the idea that users can only fetch their own notifications?
mjuniper
changed the title
AFFECTS PACKAGES: @esri/arcgis-rest-common-types @esri/arcgis-rest-users
AFFECTS PACKAGES: @esri/arcgis-rest-users
mjuniper
force-pushed
the
f/user-notifications
branch
from
6a69490
to
b4a55d0
Compare
|
Added |
| return request(url, options); | ||
| } | ||
|
|
||
| export function removeNotification( |
There was a problem hiding this comment.
Pls add comments so the generated doc has something in it :)
AFFECTS PACKAGES:
@esri/arcgis-rest-common-types
@esri/arcgis-rest-users
Thought I'd take a stab at this. Not really done-done (I would want to add createNotification and removeNotification) but I would like feedback, especially of the "OMG you did this RONG" variety.