Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(backend): ロックダウンされた期間指定のノートがStreaming経由でLTLに出現するのを修正 #15200

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

tai-cha
Copy link
Contributor

@tai-cha tai-cha commented Jan 2, 2025

What

下記現象の修正

  • LTLにストリーム経由でロックダウンされたノートがノートの内容が表示されたまま流れることがある(未来の日時を設定している等)

Why

ロックダウンしたノートが表示され続けるのは本意ではないと考えられるため

Additional info (optional)

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Jan 2, 2025
@tai-cha

This comment was marked as outdated.

Copy link

codecov bot commented Jan 2, 2025

Codecov Report

Attention: Patch coverage is 20.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 39.84%. Comparing base (3c81926) to head (0596733).
Report is 12 commits behind head on develop.

Files with missing lines Patch % Lines
...ges/backend/src/core/entities/NoteEntityService.ts 20.00% 8 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #15200      +/-   ##
===========================================
- Coverage    39.96%   39.84%   -0.13%     
===========================================
  Files         1563     1563              
  Lines       197878   197985     +107     
  Branches      3646     3635      -11     
===========================================
- Hits         79080    78878     -202     
- Misses      118193   118532     +339     
+ Partials       605      575      -30     

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

Copy link
Contributor

github-actions bot commented Jan 2, 2025

このPRによるapi.jsonの差分
差分はありません。
Get diff files from Workflow Page

@tai-cha
Copy link
Contributor Author

tai-cha commented Jan 2, 2025

いや、本質的にノートのユーザー情報とフォロー関係を見てTLのノート取得部分を変更する必要があるな
LTLに非公開なノートが配信されてしまう問題の修正に関しては一回戻して別PRに変更しよう

@tai-cha tai-cha force-pushed the fix/lockdown-with-stream branch from d3d0ccf to f96eae0 Compare January 2, 2025 02:21
@tai-cha tai-cha marked this pull request as ready for review January 2, 2025 02:23
@tai-cha tai-cha changed the title fix(backend): 期間指定のノートが一定条件でLTLに出現するのを修正 fix(backend): 期間指定のノートがStreaming経由でLTLに出現するのを修正 Jan 2, 2025
@tai-cha tai-cha changed the title fix(backend): 期間指定のノートがStreaming経由でLTLに出現するのを修正 fix(backend): ロックダウンされた期間指定のノートがStreaming経由でLTLに出現するのを修正 Jan 2, 2025
@tai-cha
Copy link
Contributor Author

tai-cha commented Jan 2, 2025

For Reviewers: Streamに乗せるときにskipHideが適用されるがこの場合visibilityの変更がされていなかったということです

const noteObj = await this.noteEntityService.pack(note, null, { skipHide: true, withReactionAndUserPairCache: true });
this.globalEventService.publishNotesStream(noteObj);

@@ -102,8 +102,7 @@ export class NoteEntityService implements OnModuleInit {
}

@bindThis
private async hideNote(packedNote: Packed<'Note'>, meId: MiUser['id'] | null): Promise<void> {
// FIXME: このvisibility変更処理が当関数にあるのは若干不自然かもしれない(関数名を treatVisibility とかに変える手もある)
private async treatVisibility(packedNote: Packed<'Note'>): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

async不要だわね

Copy link
Contributor Author

Choose a reason for hiding this comment

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

たしかにそうなのでasyncけしました

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR
Projects
Development

Successfully merging this pull request may close these issues.

2 participants