Skip to content

Conversation

Nateowami
Copy link
Collaborator

@Nateowami Nateowami commented Sep 2, 2025

The change to the lint rule is copied from #3391. I then fixed the lint issues, and made them be considered errors.


This change is Reviewable

Copy link

codecov bot commented Sep 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.21%. Comparing base (fa3da55) to head (a718c81).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3414      +/-   ##
==========================================
- Coverage   82.22%   82.21%   -0.01%     
==========================================
  Files         608      608              
  Lines       36199    36196       -3     
  Branches     5930     5911      -19     
==========================================
- Hits        29764    29759       -5     
- Misses       5562     5580      +18     
+ Partials      873      857      -16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Nateowami
Copy link
Collaborator Author

@marksvc This may be of interest to you.

@pmachapman pmachapman self-assigned this Sep 2, 2025
Copy link
Collaborator

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this by removing a chapter that contains notes (which will delete those from Mongo), and the references in sf_project_user_configs were updated correctly.

In my testing I did notice a bug (not caused by your code) where the questionRefsRead was not updated when questions were deleted, nor were the answerRefsRead and commentRefsRead arrays updated sf_project_user_configs.

@pmachapman reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Nateowami)


src/RealtimeServer/scriptureforge/services/question-service.ts line 250 at r1 (raw file):

      await this.removeEntityReadRefs(userId, docId, projectDomain, entity);
    }
    return Promise.resolve();

You can remove this line, as the async keyword will take care of this for you.

Code quote:

return Promise.resolve();

src/RealtimeServer/scriptureforge/services/note-thread-service.ts line 229 at r1 (raw file):

      await this.removeEntityHaveReadRefs(userId, docId, projectDomain, entity);
    }
    return Promise.resolve();

You can remove this line, as the async keyword will take care of this for you.

Code quote:

return Promise.resolve();

src/RealtimeServer/scriptureforge/services/note-thread-service.ts line 242 at r1 (raw file):

      await this.removeEntityHaveReadRefs(userId, docId, projectDomain, entity);
    }
    return Promise.resolve();

You can remove this line, as the async keyword will take care of this for you.

Code quote:

return Promise.resolve();

Copy link
Collaborator

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have logged the bug I found as SF-3546.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Nateowami)

@Nateowami Nateowami force-pushed the lint/realtime-server-promises branch from 3749fc0 to 87019bd Compare September 3, 2025 12:21
Copy link
Collaborator Author

@Nateowami Nateowami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 2 of 4 files reviewed, 3 unresolved discussions (waiting on @pmachapman)


src/RealtimeServer/scriptureforge/services/note-thread-service.ts line 229 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

You can remove this line, as the async keyword will take care of this for you.

Good point. Done.


src/RealtimeServer/scriptureforge/services/note-thread-service.ts line 242 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

You can remove this line, as the async keyword will take care of this for you.

Done.


src/RealtimeServer/scriptureforge/services/question-service.ts line 250 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

You can remove this line, as the async keyword will take care of this for you.

Done.

@pmachapman pmachapman force-pushed the lint/realtime-server-promises branch from 87019bd to a718c81 Compare September 3, 2025 19:11
Copy link
Collaborator

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@pmachapman reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Nateowami)

@pmachapman pmachapman merged commit 17e6e6e into master Sep 3, 2025
23 checks passed
@pmachapman pmachapman deleted the lint/realtime-server-promises branch September 3, 2025 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants