FoundKeyGang/FoundKey
FoundKeyGang/FoundKey
7
19
Fork 22
Code Issues 37 Pull requests 4 Releases Activity

backend: Fix various lints in services/note #206

Merged
norm merged 6 commits from backend-services-note into main 2022-10-21 19:29:41 +00:00
Conversation 0 Commits 6 Files changed 21 +113 -117
norm commented 2022-10-16 21:53:05 +00:00
Owner
Copy link
No description provided.
Johann150 reviewed 2022-10-17 17:52:53 +00:00
packages/backend/src/services/note/create.ts Outdated
@ -228,3 +229,2 @@
if (data.reply && !data.visibleUsers.some(x => x.id === data.reply!.userId)) {
data.visibleUsers.push(await Users.findOneByOrFail({ id: data.reply!.userId }));
if (data.reply && !data.visibleUsers.some(x => x.id === data.reply?.userId)) {
Johann150 commented 2022-10-17 17:26:36 +00:00
Owner
Copy link

I don't understand why you made this one into a question mark. Doesnt the previous term of the && make sure that data.reply is not null? (similar to line 217)

I don't understand why you made this one into a question mark. Doesnt the previous term of the `&&` make sure that `data.reply` is not null? (similar to line 217)
norm commented 2022-10-17 18:12:09 +00:00
Author
Owner
Copy link

For whatever reason TS doesn't really see that data.reply is non-null in the first term from within the lambda in data.visibleUsers.some.

I could split up the if statement, but that would be kinda ugly, or otherwise use a non-null assert with // eslint-disable-next-line @typescript-eslint/no-non-null-assertion maybe...

For whatever reason TS doesn't really see that `data.reply` is non-null in the first term from within the lambda in `data.visibleUsers.some`. I could split up the `if` statement, but that would be kinda ugly, or otherwise use a non-null assert with `// eslint-disable-next-line @typescript-eslint/no-non-null-assertion` maybe...
Michcio commented 2022-10-17 19:27:15 +00:00
Contributor
Copy link

It might not be smart enough to figure out a guarantee that the lambda will be used immediately, I guess?

It might not be smart enough to figure out a guarantee that the lambda will be used immediately, I guess?
norm commented 2022-10-17 20:19:06 +00:00
Author
Owner
Copy link

Yeah that appears to be the case: https://github.com/microsoft/TypeScript/issues/34795

Yeah that appears to be the case: https://github.com/microsoft/TypeScript/issues/34795
Johann150 commented 2022-10-18 17:45:49 +00:00
Owner
Copy link

Okay then I guess its gonna stay that way. Seems you already made those two lines the same at least.

Okay then I guess its gonna stay that way. Seems you already made those two lines the same at least.
Johann150 marked this conversation as resolved
packages/backend/src/services/note/create.ts Outdated
@ -502,2 +503,2 @@
id: genId(data.createdAt!),
createdAt: data.createdAt!,
id: genId(data.createdAt ?? undefined),
createdAt: data.createdAt ?? undefined,
Johann150 commented 2022-10-17 17:46:34 +00:00
Owner
Copy link

I think more sensible would be to default to the current time.

createdAt: data.createdAt ?? new Date(),
I think more sensible would be to default to the current time. ```typescript createdAt: data.createdAt ?? new Date(), ```
norm marked this conversation as resolved
packages/backend/src/services/note/create.ts Outdated
@ -559,0 +556,4 @@
choices: data.poll?.choices,
expiresAt: data.poll?.expiresAt,
multiple: data.poll?.multiple,
votes: new Array(data.poll?.choices.length).fill(0),
Johann150 commented 2022-10-17 17:31:53 +00:00
Owner
Copy link

I think here it might make sense to check for data.poll != null and disregarding data.hasPoll? At least that feels more sensible to me. Perhaps remove hasPoll here alltogether because it doesn't make sense when you can set data.poll to null.

I think here it might make sense to check for `data.poll != null` and disregarding `data.hasPoll`? At least that feels more sensible to me. Perhaps remove `hasPoll` here alltogether because it doesn't make sense when you can set `data.poll` to `null`.
norm marked this conversation as resolved
packages/backend/src/services/note/delete.ts Outdated
@ -45,3 +45,3 @@
if (isPureRenote(note)) {
renote = await Notes.findOneBy({
id: note.renoteId,
id: note.renoteId ?? undefined,
Johann150 commented 2022-10-17 17:41:10 +00:00
Owner
Copy link

This neither looks like a good idea (I think it would just pick a random note?) nor sensible, since isPureRenote checks among other things that note.renoteId != null.

This neither looks like a good idea (I think it would just pick a random note?) nor sensible, since [`isPureRenote` checks among other things that `note.renoteId != null`](https://akkoma.dev/FoundKeyGang/FoundKey/src/commit/04d4dd323f43992f08be038d0583e3a5f03e9f44/packages/backend/src/misc/renote.ts#L4).
norm marked this conversation as resolved
norm force-pushed backend-services-note from 6046a58276 to 8d0219a507 2022-10-17 20:58:29 +00:00 Compare
norm force-pushed backend-services-note from 8d0219a507 to 820c47e944 2022-10-19 22:42:45 +00:00 Compare
norm force-pushed backend-services-note from f24cd29099 to 246f47c994 2022-10-21 01:16:44 +00:00 Compare
norm force-pushed backend-services-note from 246f47c994 to 987dd203b1 2022-10-21 01:23:02 +00:00 Compare
norm force-pushed backend-services-note from cb29c2cb3b to 43644494d3 2022-10-21 17:33:10 +00:00 Compare
norm merged commit c36cca30cb into main 2022-10-21 19:29:41 +00:00
norm deleted branch backend-services-note 2022-10-21 19:29:41 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
feature

task that proposes new feature(s)

  
fix

task to fix behavioral problems in the FoundKey codebase

  
upkeep

task that should make FoundKey smaller and more maintainable

No labels
feature
fix
upkeep
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: FoundKeyGang/FoundKey#206
No description provided.
Delete branch "backend-services-note"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?