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
add back sessionDate to api call. #285
Conversation
src/core/api/user-permissions.js
Outdated
* sessionDate used by api to determine if server side cache is out of date. | ||
* i.e if user cache on server is older then sessionDate it will repull user information. | ||
*/ | ||
this.sessionDate = moment().toISOString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will cache the sessionDate
to when the instance of AvUserPermissionsResourceFactory
is instantiated. All of the calls afterwards would have the same sessionDate
. Is this what is wanted or should it create a new sessionDate
value when the calls is made (or eventually expire)?
A problem I could see happening is if this sessionDate
is old (the user has been on the page for a little while) and this is older than the cache on the server this will trigger the server to not repull even though the cache on the server was out-of-date. And if that is the case, you would be better off caching the inital results in memory on the client (not saying that is a good idea, just saying that it would avoid unneeded network calls since you could determine that the server would return the same cached results).
Also, new Date().toJSON()
does the same thing and is native to the browser.
moment
tends to get overused and while it is nice for parsing, comparing, and fancy formatting and is already included in the project, it would be nice to only use it when the browser doesn't already natively support the feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior is as desired that only bust cache on server potentially on initial load. Subsequent calls would hit cache. I agree that is essentially same as caching on client, but currently not sure good way to change back in logic without significant risk. This change was how it was working before in 1.x version due to issues with switching region and server not having the must current region for user. I will make change to new Date()
http://availity.github.io/availity-angular/docs/src/core/api/ could have leveraged |
In the issue (#284) sounded like the API itself was looking for |
ah good catch |
No description provided.