Skip to content
This repository has been archived by the owner on Apr 12, 2023. It is now read-only.

Suppress notification if the exposure date time is expired. #977

Merged

Conversation

keiji
Copy link
Collaborator

@keiji keiji commented Apr 17, 2022

Issue 番号 / Issue ID

目的 / Purpose

  • 接触が14日より前であれば通知を出さないようにする

変更内容 / Changes

  • AppConstants.DaysOfExposureInformationToDisplayの値でフィルターを追加

破壊的変更をもたらしますか / Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request の種類 / Pull Request type

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

確認事項 / What to check

  • 実際に接触を発生させて14日待つテストはつらいので、境界値テストはユニットテストでカバー。実機テストは通常の接触が問題なく通知されるかのテストに限定してはどうでしょう

その他 / Other information


Internal IDs:

@keiji keiji changed the title Suppress notification if the exposure date time is past 14 days ago. Suppress notification if the exposure date time is pastDaysOfExposureInformationToDisplay days ago. Apr 17, 2022
@keiji keiji changed the title Suppress notification if the exposure date time is pastDaysOfExposureInformationToDisplay days ago. Suppress notification if the exposure date time is past DaysOfExposureInformationToDisplay days ago. Apr 17, 2022
@keiji keiji marked this pull request as ready for review April 17, 2022 02:10
@keiji keiji self-assigned this Apr 17, 2022
@b-wind
Copy link

b-wind commented Apr 17, 2022

「通知」の閾値としても使用するなら、 DaysOfExposureInformationToDisplay と言う変数名は変えた方が良い様に思います。

@keiji
Copy link
Collaborator Author

keiji commented Apr 18, 2022

ありがとうございます。46e161e と 1c085ec で対応しました。
定数名をなににするかはけっこう悩んだので、もっと良い名前があれば提案ください。

@keiji keiji changed the title Suppress notification if the exposure date time is past DaysOfExposureInformationToDisplay days ago. Suppress notification if the exposure date time is expired. Apr 18, 2022
@b-wind
Copy link

b-wind commented Apr 18, 2022

変数(定数)名は私もいつも頭を悩ませるのですが、こういうのはどうでしょうか?
型も変えてしまっていますが。明確に日数が必要なときは TimeSpan.Days を呼べば良いかなと思ったので。

var RetentionPeriodOfExposureInformation = new TimeSpam(14,0,0,0);

「データ保持期間」的な意味合いですが、実際には消えてない・消してないので誤解を生むかも?

@keiji
Copy link
Collaborator Author

keiji commented Apr 28, 2022

変数(定数)名は私もいつも頭を悩ませるのですが、こういうのはどうでしょうか? 型も変えてしまっていますが。明確に日数が必要なときは TimeSpan.Days を呼べば良いかなと思ったので。

var RetentionPeriodOfExposureInformation = new TimeSpam(14,0,0,0);

「データ保持期間」的な意味合いですが、実際には消えてない・消してないので誤解を生むかも?

これ反応が遅くなりました。
そうですね。フィルター条件という扱いなのでRetantionだと個人的に違和感があります。
一回取り込んで、必要に応じてリファクタリングかけますね。

@cocoa-dev003 cocoa-dev003 merged commit 2ce01b1 into cocoa-mhlw:develop Apr 28, 2022
@keiji keiji deleted the suppress_unnecessary_notification branch June 15, 2022 04:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants