Warn offline users before replicating too many docs #5793
Conversation
dianabarsan
force-pushed
the
5362-offline-user-info
branch
from
4814e3b
to
5474f12
Compare
|
@garethbowen could you, please, have a look? Thank you! |
| isOffline: roles => { | ||
| const configured = config.get('roles') || {}; | ||
| return roles.some(role => configured[role] && configured[role].offline); | ||
| }, |
There was a problem hiding this comment.
It'd be good to have some code reuse between this and isOnlineOnly - they do the exact opposite thing I think?
Maybe change isOnlineOnly to take an array of roles?
There was a problem hiding this comment.
Well, they don't do the exact opposite thing.
isOnlineOnly checks for the mm-online flag-role in a provided role list part of a userCtx, regardless of roles config, while isOffline checks config to see if the provided roles are configured as offline.
Do you think it would be more correct for isOnlineOnly to check config - and the mm-online flag to only be useful in webapp while bootstrapping?
There was a problem hiding this comment.
Do you think it would be more correct for isOnlineOnly to check config - and the mm-online flag to only be useful in webapp while bootstrapping?
Yes I think so. That way they're consistent as much as possible.
There was a problem hiding this comment.
I had a bit of a philosophical dilemma about this: if a user had no configured role, is it online or offline? One can easily get to this point if a role is created, a user with this role is created, role is removed.
I decided that one such user should be considered offline and updated the isOffline function to return true if none of the provided roles are configured, along with doing a !isOffline in the isOnlineOnly. But I'm not convinced this is correct.
There was a problem hiding this comment.
I think in that case it should fail before it gets here, ie: every request should 403 if you don't have any known role, because you're not permitted to do anything! Feel free to go with what you have this time, and maybe raise an issue for that edge case?
| @@ -81,6 +81,7 @@ const request = (options, { debug, noAuth, notJson } = {}) => { | |||
|
|
|||
| const err = new Error(errorMessage); | |||
| err.responseBody = body; | |||
| err.statusCode = res.statusCode; | |||
Yes, that's the plan!
Absolutely, going to start working on that. |
dianabarsan
force-pushed
the
5362-offline-user-info
branch
from
4be4fbe
to
fb92f81
Compare
dianabarsan
force-pushed
the
5362-offline-user-info
branch
from
fb92f81
to
007d2ae
Compare
Description
Adds
/api/v1/users-infoendpoint whichwarnflag if this number exceeds the recommended limit.facility_idandroleparams, plus an optionalcontact_idparam (either GET or POST).webappbootstrapper. When resultingwarnflag is truthy, the user has to confirm to actually continue bootstrapping and start replication. Cancelling redirects to login.adminuser creation/updating. When resultingwarnflag is truthy, a warning message is displayed. The admin would need to click on "Submit" the 2nd time to actually create or update the user document.Once an hour, API will log warnings of users that have replicated more than 10 000 docs.
Updates
webappbootstrapper to display correct doc count progression, even when interrupted.#5362
Code review items
License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.