feat: Add support for filtering encrypted field by list of values#120
feat: Add support for filtering encrypted field by list of values#120SteveKekacs wants to merge 2 commits into47ng:nextfrom
Conversation
Pull Request Test Coverage Report for Build 10309403851Details
💛 - Coveralls |
SteveKekacs
changed the title
franky47
left a comment
franky47
left a comment
There was a problem hiding this comment.
Thanks for this feature!
A couple of things could do with some changes (field naming and checking for arrays), otherwise it looks good.
| function encryptFieldValue({ | ||
| fieldConfig, | ||
| value: clearText, | ||
| value: unHashedValue, |
There was a problem hiding this comment.
suggestion: Keep the original clearText name, as the value is not only used for hashing, but also for encryption in most (writing) cases.
| if (typeof unHashedValue === 'object') { | ||
| hash = (unHashedValue as string[]).map((value) => hashString(value, fieldConfigHash)); |
There was a problem hiding this comment.
suggestion: Use the Array.isArray function, to make the intent clearer. It also removes the need for the as string[] type casting.
|
|
||
| // Encrypting list values is not yet supported | ||
| if (typeof unHashedValue !== 'string') { | ||
| return; |
There was a problem hiding this comment.
suggestion: Could we add a log point to this code path, for debugging? It's unlikely we'll hit it with Prisma's current API AFAIK, but it would make it easier to catch issues in the future.
| ( | ||
| // Used for where: { field: in: []} queries | ||
| typeof (node as any)?.[specialSubField] === 'string' || | ||
| typeof (node as any)?.[specialSubField] === 'object' |
There was a problem hiding this comment.
suggestion: Same test here, using Array.isArray.
- Apply Prettier formatting
|
@SteveKekacs Can we please move forward with this PR? If you do not have time I could take over and apply changes from comments. I really need this feature in my project. Thanks. |
Hi @m-przybylski, apologies, I'm no longer using this library so go ahead and take over. |
|
Superseded by #136. |
4 participants
Took a stab at implementing a fix for #119.