Feat/soft delete #166
Feat/soft delete #166
Conversation
…sagi/release/0.2.0
destructuring function argument Co-authored-by: Snir Shechter <me@snir.sh>
Co-authored-by: Snir Shechter <me@snir.sh>
Co-authored-by: Snir Shechter <me@snir.sh>
Co-authored-by: Snir Shechter <me@snir.sh>
throwing an error when non-privileged user trying to create a custom id
Codecov Report
@@ Coverage Diff @@
## release/0.2.0 #166 +/- ##
=================================================
- Coverage 17.52% 16.98% -0.55%
=================================================
Files 40 41 +1
Lines 1335 1378 +43
Branches 50 51 +1
=================================================
Hits 234 234
- Misses 1069 1111 +42
- Partials 32 33 +1
Continue to review full report at Codecov.
|
| @@ -69,15 +70,15 @@ export class RelationalStorage implements StorageDriver { | |||
| url = new (class RelationalUrlStorage { | |||
| constructor(public storage: RelationalStorage) { } | |||
|
|
|||
| public async get(id: string, options = { withInfo: false }): Promise<StoredUrl | UrlWithInformation> { | |||
| public async get(id: string, options = { withInfo: false, isAuthorized: false }): Promise<StoredUrl | UrlWithInformation> { | |||
There was a problem hiding this comment.
The storage service should not be aware of any authorization checks. Change isAuthorized to a different name (probably something such as includeDeleted or so).
There was a problem hiding this comment.
Took the name you suggested
| .select('*') | ||
| .where('id', id) | ||
| .first() | ||
| if (storedUrl?.deletedAt && !options.isAuthorized) throw new UnauthorizedError(); |
There was a problem hiding this comment.
If a user asks for a URL with id=4 (doesn't exist), it'll receive a 404. If they request a URL with id=3 (deleted), they'll receive an UnauthorizedError. This gives away the fact we use soft-delete instead of hard-delete (bad).
| await this.storage.db.table<StoredUrl>('urls').where('id', id).delete() | ||
| } else { | ||
| await this.storage.db.table<StoredUrl>('urls').update({ deletedAt: new Date().toISOString() }).where('id', id) | ||
| } | ||
| } | ||
|
|
||
| public async deleteOverdue(timespanMs: number): Promise<number> { |
There was a problem hiding this comment.
What about deleteOverdue? We need it to soft-delete too.
There was a problem hiding this comment.
added a deleteOverdue + migration to support the soft delete in postgres
- Add a migration for the new 'deletedAt' Column - Define a default SOFT delete method for deleteOverdue - Change the 'options' parameter passed to the drivers' functions
Hi, Snir was updated yesterday, it was WIP, now can be reviewed again, thanks for the heads up! |
Hi,
I added the option for soft for both InMemory and Relational, need to add Redis
I made it optional, and also I prevent getting the URL if the URL was soft deleted, please let me know what you think
I've merged the release branch to prevent conflicts in the future, that made the PR a bit hard to follow, my new Impl starts at:
"Merge remote-tracking branch 'origin/release/0.2.0' into release/0.2.0"
#79