feat(classification): add watermarking to security controls) - #2807
Conversation
a02502a to
a18be37
Compare
| accessLevel: SharedLinkAccessLevel, | ||
| }; | ||
|
|
||
| type watermarkApplied = { |
There was a problem hiding this comment.
Would it be preferable to name this consistently like WatermarkRestriction, or is there a reason why a different convention was used in this case?
There was a problem hiding this comment.
Was discussing with Yuvnesh on this, watermarking is not considered as a restriction like the other controls, though we have it as part of access policy, which is also why we are printing it with an extra bullet point in the short version.
|
|
||
| const getWatermarkingMessages = (controls: Controls): Array<MessageItem> => { | ||
| const items = []; | ||
| const isWatermarkEnabled = getProp(controls, `${WATERMARK}.enabled`); |
There was a problem hiding this comment.
nit: Might be good to add a default false value, in case this object is missing
|
|
||
| if (download) { | ||
| return { message: messages.shortDownload }; | ||
| switch (true) { |
There was a problem hiding this comment.
I feel like this was a bit simpler before. I think ideally the switch would be used for enums and each case would match a unique value, but here all cases resolve to true. I think the same logic should be possible with the existing ifs.
There was a problem hiding this comment.
I was thinking about this too, using existing ifs we would now gotta concat each with else ifs since we don't return immediately, looks a bit bulky. Let me know how you think ?
There was a problem hiding this comment.
I think the else if would be easier to understand and would result in less lines of code. A switch of boolean values where all the cases resolve to true is not a common usage and feels like it could be error prone.
|
|
||
| if (app) { | ||
| return { message: messages.shortApp }; | ||
| if (watermark) { |
There was a problem hiding this comment.
So far we've only displayed a single bullet for the short version of the summary. Just to confirm: does product/design want to make an exception for watermarking?
There was a problem hiding this comment.
Yes confirm this is desired with Product.
a18be37 to
ff36f12
Compare
ff36f12 to
a7f6498
Compare
2 participants
![Screen Shot 2022-01-19 at 10 47 34 PM](h