Rework classes to anonymize users in the background#292
Conversation
src/models/Site.js
Outdated
| // extract externalUserIds | ||
| result.externalUserIds = result.users.filter( user => user.externalUserId ).map( user => user.externalUserId ); | ||
|
|
||
| console.log({result}); |
There was a problem hiding this comment.
Een verdwaalde console.log
src/models/Site.js
Outdated
| let users = await db.User.findAll({ where: { externalUserId } }); | ||
| if (users.length == 0) { | ||
| // no api users left for this oauth user, so remove the oauth user | ||
| let which = req.query.useOauth || 'default'; |
There was a problem hiding this comment.
req.query heb je hier niet. Net zo min als req.site en req.params hieronder.
There was a problem hiding this comment.
Wat is overigens de reden dat je dat nar hier hebt verplaatst? Ik vermoed omdat het in principe netter is om het in het model te doen, maar de reden om het niet hier te doen was dat je nu de instantie Site=12 verantwoordelijk maakt voor users die niet onder site 12 vallen.
Voor beide valt wat te zeggen denk ik....
There was a problem hiding this comment.
Oh, ik zie nu dat je de req meestuurt. Zou je daar losse params van kunnen maken? req is een concept dat niet in een model thuis hoort denk ik.
There was a problem hiding this comment.
Wat is overigens de reden dat je dat nar hier hebt verplaatst? Ik vermoed omdat het in principe netter is om het in het model te doen, maar de reden om het niet hier te doen was dat je nu de instantie Site=12 verantwoordelijk maakt voor users die niet onder site 12 vallen. Voor beide valt wat te zeggen denk ik....
De users worden opgehaald aan de hand van de result variabele die uit willAnonyizeAllUsers komt. Deze filtert de users op o.a. de siteId van de betreffende site. De doAnonymizeAllUsers wordt slechts op 1 plek gebruikt met deze gefilterde variabele dus users die niks te maken hebben met deze site zullen niet worden geanonymiseerd.
Het is uiteraard wel mogelijk om zo een scenario in te programmeren als je een andere lijst aan users meegeeft aan doAnonymizeAllUsers. Dit is dan wel iets dat je bewust zal programmeren.
Als laatste ben ik van mening dat door het aanreiken van een user lijst aan de functie, in plaats van willAnonyizeAllUsers aanroepen binnen doAnonymizeAllUsers, zorg je ervoor dat mocht willAnonyizeAllUsers veranderen dan hoef je niet de methode doAnonymizeAllUsers aan te passen maar alleen mogelijk de doorgegeven waarde te wijzigen
|
Dit lijkt me prima zo. Heb ik wel een vervolg verzoek: kun je kijken of er iets van feedback mogelijk is. De netste oplossing zou zijn om eea via actions te laten werken, maar voor nu misschien een beetje overkill. Voor het genereren van unieke ccodes gebeurd dat in de oauth server. Die heeft wat extra routes om de status van het genereren op te vragen. Nu is dat geen ideale oplossing, maar kijk er eens naar en misschien kun je wat leuks bedenken... |
* render concept templates for idea when creating or when changed from concept to published * Added concept templates * Add check on existing users bofore deleting a site * Changed mail implementation to allow changing the email in the admin panel * Added a migration to add type column and exposed it * Rework classes to anonymize users in the background (#292) * Rework classes to anonymize users in the background * remove console.log statement * Remove task code which does not really seem usefull in any way * Remove the do check inside method doAnonymizeAllUsers, it will always be in 'do' replace req param with self * Remove dangling console log statement * Added length check for checking if there are any users left on the side - [] is truthy (#294) * Added type field when creating a tag (#295) * Feature/add conceptmail and concept topublished config (#296) * Added conceptMail and conceptToPublishedEmail config * Remove accidental .env --------- Co-authored-by: Niels Vegter <niels@denes.nl>
* render concept templates for idea when creating or when changed from concept to published * Added concept templates * Add check on existing users bofore deleting a site * Changed mail implementation to allow changing the email in the admin panel * Added a migration to add type column and exposed it * Rework classes to anonymize users in the background (#292) * Rework classes to anonymize users in the background * remove console.log statement * Remove task code which does not really seem usefull in any way * Remove the do check inside method doAnonymizeAllUsers, it will always be in 'do' replace req param with self * Remove dangling console log statement * Added length check for checking if there are any users left on the side - [] is truthy (#294) * Added type field when creating a tag (#295) * Feature/add conceptmail and concept topublished config (#296) * Added conceptMail and conceptToPublishedEmail config * Remove accidental .env * Added new regex to also support {ideaId} format used in global url& api instellingen slug setting (#300) --------- Co-authored-by: Niels Vegter <niels@denes.nl>
2 participants
Anonimizing the users of a site on the background.
I have taken an example of the class auth/middleware/code.js