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

DebugPage の実装 #178

Merged
merged 26 commits into from
Sep 2, 2021
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
92fb986
new for issue#160
i-maruyama May 14, 2021
c22df7f
issue#160 (simple debugpage)
i-maruyama May 14, 2021
47f32e3
Update Covid19Radar/Covid19Radar/ViewModels/HomePage/DebugPageViewMod…
i-maruyama May 14, 2021
baa112f
Update Covid19Radar/Covid19Radar/ViewModels/HomePage/DebugPageViewMod…
i-maruyama May 14, 2021
516d974
Update Covid19Radar/Covid19Radar/ViewModels/HomePage/DebugPageViewMod…
i-maruyama May 14, 2021
adc6bfe
Update Covid19Radar/Covid19Radar/ViewModels/HomePage/DebugPageViewMod…
i-maruyama May 14, 2021
77209c1
refactoring and delete unused services
i-maruyama May 14, 2021
e9bd943
change name of variables
i-maruyama May 14, 2021
239f33c
Update Covid19Radar/Covid19Radar/ViewModels/HomePage/DebugPageViewMod…
i-maruyama May 15, 2021
39e03a3
follow VisualStudio's advice. c.f.: https://github.com/cocoa-mhlw/coc…
i-maruyama May 15, 2021
3857b4a
`Release` ビルド時にデバッグページを除外して容量を削減する
Takym May 16, 2021
b8f90a3
XAMLファイルが正しく除外されていなかったので修正した
Takym May 16, 2021
9273bcb
git mv HomePage/... Settings/ c.f.: https://github.com/cocoa-mhlw/co…
i-maruyama May 16, 2021
80128f5
Info への変更
i-maruyama May 16, 2021
7d64bc3
Update Covid19Radar/Covid19Radar/ViewModels/Settings/DebugPageViewMod…
i-maruyama May 16, 2021
4f594a2
Update Covid19Radar/Covid19Radar/ViewModels/Settings/DebugPageViewMod…
i-maruyama May 16, 2021
8fbbc10
Update Covid19Radar/Covid19Radar/ViewModels/Settings/DebugPageViewMod…
i-maruyama May 16, 2021
2185ec7
Merge pull request #1 from Takym/refactoring/issue-160
i-maruyama May 25, 2021
857bc1c
directory name fix
i-maruyama May 25, 2021
da3f2c1
Revert "directory name fix"
i-maruyama May 26, 2021
051ab33
commit only Covid19Radar.csproj
i-maruyama May 26, 2021
243a3d3
Merge branch 'cocoa-mhlw:develop' into issue#160
i-maruyama Jun 7, 2021
de97c2e
use GetExposureCountToDisplay.
i-maruyama Jun 7, 2021
0bda3cf
Merge branch 'cocoa-mhlw:develop' into issue#160
i-maruyama Jun 9, 2021
489700c
Merge branch 'feature/debug_page' of github.com:cocoa-mhlw/cocoa into…
keiji Sep 2, 2021
24be80f
Merge pull request #2 from keiji/issue#160
i-maruyama Sep 2, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Covid19Radar/Covid19Radar/App.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ protected override void RegisterTypes(IContainerRegistry containerRegistry)
containerRegistry.RegisterForNavigation<NavigationPage>();
containerRegistry.RegisterForNavigation<MenuPage>();
containerRegistry.RegisterForNavigation<HomePage>();
#if DEBUG
containerRegistry.RegisterForNavigation<DebugPage>();
#endif

// Settings
containerRegistry.RegisterForNavigation<SettingsPage>();
Expand Down
7 changes: 6 additions & 1 deletion Covid19Radar/Covid19Radar/Covid19Radar.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,9 @@
<ItemGroup>
<ProjectReference Include="..\Xamarin.ExposureNotification\Xamarin.ExposureNotification.csproj" />
</ItemGroup>
</Project>
<ItemGroup Condition="'$(Configuration)'=='Release'">
<Compile Remove="ViewModels/Settings/DebugPageViewModel.cs" />
<Compile Remove="Views/Settings/DebugPage.xaml.cs" />
<EmbeddedResource Remove="Views/Settings/DebugPage.xaml" />
</ItemGroup>
</Project>
10 changes: 10 additions & 0 deletions Covid19Radar/Covid19Radar/ViewModels/MenuPageViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ public MainMenuModel SelectedMenuItem
public MenuPageViewModel(INavigationService navigationService) : base(navigationService)
{
MenuItems = new ObservableCollection<MainMenuModel>();
#if DEBUG
MenuItems.Add(new MainMenuModel()
{
Icon = "\uf013",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

本質的ではないのですが、虫Icon があるようですね。そちらにしたくなってきました。

PageName = nameof(DebugPage),
Title = "debug",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Title = "debug",
Title = "Debug",

IconColor = "#019AE8",
TextColor = "#000"
});
#endif
Comment on lines +32 to +41
Copy link
Collaborator

Choose a reason for hiding this comment

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

デバッグ画面のカラムは一番下に置きましょう(あまり主張したくないので)。

MenuItems.Add(new MainMenuModel()
{
Icon = "\uf965",
Expand Down
160 changes: 160 additions & 0 deletions Covid19Radar/Covid19Radar/ViewModels/Settings/DebugPageViewModel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */

using System;
using Acr.UserDialogs;
using Covid19Radar.Services;
using Prism.Navigation;
using Xamarin.Forms;

namespace Covid19Radar.ViewModels
{
public class DebugPageViewModel : ViewModelBase
{
private readonly IUserDataService userDataService;
private readonly ITermsUpdateService termsUpdateService;
private readonly IExposureNotificationService exposureNotificationService;

private string _debugInfo;
public string DebugInfo
{
get { return _debugInfo; }
set { SetProperty(ref _debugInfo, value); }
}
public async void Info(string ex = "")
Copy link
Collaborator

Choose a reason for hiding this comment

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

メソッド名は動詞から始めてください。また、asyncキーワードがついている場合は接尾辞にAsyncを付けてください。

今回の場合、表示する情報を更新する役割のメソッドなのでUpdateInfoAsyncでいかがでしょうか。

Copy link
Collaborator

Choose a reason for hiding this comment

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

引数名exは何を意味していますか? 略称ではなく、実態に沿った名前にしてもらえるとわかりやすくてありがたいです。

{
string os;
switch (Device.RuntimePlatform)
{
case Device.Android:
os = "Android";
break;
case Device.iOS:
os = "iOS";
break;
default:
os = "unknown";
break;
}
Comment on lines +27 to +39
Copy link
Collaborator

Choose a reason for hiding this comment

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

Device.RuntimePlatformをそのまま使えば、OSプラットフォームの文字列をとることができます。Windowsとかも入ってしまいますが、そもそもビルドできないので考慮しなくて良いと思います。

Suggested change
string os;
switch (Device.RuntimePlatform)
{
case Device.Android:
os = "Android";
break;
case Device.iOS:
os = "iOS";
break;
default:
os = "unknown";
break;
}
string os = Device.RuntimePlatform;

#if DEBUG
os += ",DEBUG";
#endif
#if USE_MOCK
os += ",USE_MOCK";
#endif

// debug info for ./SplashPageViewModel.cs
string agree;
if (termsUpdateService.IsAllAgreed())
{
agree = "exists";// (mainly) navigate from SplashPage to HomePage
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
agree = "exists";// (mainly) navigate from SplashPage to HomePage
agree = "All agreed"; // (mainly) navigate from SplashPage to HomePage

var termsUpdateInfo = await termsUpdateService.GetTermsUpdateInfo();
if (termsUpdateService.IsReAgree(TermsType.TermsOfService, termsUpdateInfo))
{
agree += "-TermsOfService";
}
else if (termsUpdateService.IsReAgree(TermsType.PrivacyPolicy, termsUpdateInfo))
{
agree += "-PrivacyPolicy";
}
Comment on lines +52 to +60
Copy link
Collaborator

Choose a reason for hiding this comment

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

ここはtermsUpdateInfoに含まれているTermsOfServicePrivacyPolicyの最新の値(更新日付?)を表示する方が良いかなと思いました。

ここは「利用許諾」と「プライバシーポリシー」が更新されている経路だと理解しています。

一般的にデバッグ画面に到達したときにはすでプライバシーポリシーの再同意が行われている状態になるので、-TermsOfService-PrivacyPolicyがつくことは極めて稀(実行中に規約類が更新されるタイミング?)なので、前述通り、TermsOfServicePrivacyPolicyの最新の値(更新日付?)を表示する方が良いかと思います。

}
else
{
agree = "not exists"; // navigate from SplashPage to TutorialPage1
Copy link
Collaborator

Choose a reason for hiding this comment

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

デバッグ画面に到達するには初期設定が必要と理解しているので、ここの経路に到達することはないかと思いますが、仮にこの経路があるとすれば、

Suggested change
agree = "not exists"; // navigate from SplashPage to TutorialPage1
agree = "Not agreed"; // navigate from SplashPage to TutorialPage1

}

var ticks = exposureNotificationService.GetLastProcessTekTimestamp(AppSettings.Instance.SupportedRegions[0]);
var dt = DateTimeOffset.FromUnixTimeMilliseconds(ticks).ToOffset(new TimeSpan(9, 0, 0));
//please check : offset is correct or not
//cf: ../../../Covid19Radar.Android/Services/Logs/LogPeriodicDeleteServiceAndroid.cs
var lastProcessTekTimestamp = dt.ToLocalTime().ToString("F");
Comment on lines +67 to +71
Copy link
Collaborator

Choose a reason for hiding this comment

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

GetLastProcessTekTimestampで得た値なのでticksよりlastProcessTekTimestampの変数に入れた方が良いと思います。あとのlastProcessTekTimestampについてはDateTimeを意味しているのでlastProcessTekDateTimeとするのはどうでしょうか。

また、取得した日時に対してToOffsetで9時間ずらしているようですが、これは後段のToLocalTime()で足りるような気がします。

            var lastProcessTekTimestamp = exposureNotificationService.GetLastProcessTekTimestamp(AppSettings.Instance.SupportedRegions[0]);
            var lastProcessTekDateTime = DateTimeOffset.FromUnixTimeMilliseconds(lastProcessTekTimestamp).ToLocalTime().ToString("F");


var exposureNotificationStatus = await Xamarin.ExposureNotifications.ExposureNotification.IsEnabledAsync();
var exposureNotificationMessage = await exposureNotificationService.UpdateStatusMessageAsync();
// ../../settings.json
var str = new[] { "Build: " + os, "Ver: " + AppSettings.Instance.AppVersion,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

setting.json から設定される AppSettings.Instance.AppVersion,Write  は、意味のない情報のようで、ログや、バージョンチェックで使われるのは, EssentialsService.AppVersion のようですね。
何を表示させるか?は、もう少し議論が必要だと思います。

Copy link
Collaborator

Choose a reason for hiding this comment

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

表示するのはEssentialsService.AppVersionにしましょう。

Copy link
Collaborator

Choose a reason for hiding this comment

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

文字列連結は + ではなく、$を付けた変数を展開する記法を使ってください。
$"Build: {os}, Version: {EssentialsService.AppVersion}" のようになります。

あと、VerVersionにするなど、短い単語であれば可能な限り略称を使わないようにしてください。

"Region: " + string.Join(",", AppSettings.Instance.SupportedRegions), "CdnUrl: " + AppSettings.Instance.CdnUrlBase,
"ApiUrl: " + AppSettings.Instance.ApiUrlBase, "Agree: " + agree, "StartDate: " + userDataService.GetStartDate().ToLocalTime().ToString("F"),
"DaysOfUse: " + userDataService.GetDaysOfUse(), "ExposureCount: " + exposureNotificationService.GetExposureCountToDisplay(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

v1.2.4 に対応するため、GetExposureCountToDisplayにしました。

"LastProcessTek: " + lastProcessTekTimestamp, " (long): " + ticks, "ENstatus: " + exposureNotificationStatus,
"ENmessage: " + exposureNotificationMessage, "Now: " + DateTime.Now.ToLocalTime().ToString("F"), ex};
DebugInfo = string.Join(Environment.NewLine, str);
}
public DebugPageViewModel(INavigationService navigationService, IUserDataService userDataService, ITermsUpdateService termsUpdateService, IExposureNotificationService exposureNotificationService) : base(navigationService)
Copy link
Collaborator

Choose a reason for hiding this comment

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

横に長くなっているので適切に改行+インデントをお願いします。

{
Title = "Title:DebugPage";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Title = "Title:DebugPage";
Title = "Title:Debug";

Debug Pageでも良いかと思いましたが、開発者しか使わないことを考えると、よりシンプルにDebugでいいかと思います。

this.userDataService = userDataService;
this.termsUpdateService = termsUpdateService;
this.exposureNotificationService = exposureNotificationService;
}
public override void Initialize(INavigationParameters parameters)
{
base.Initialize(parameters);
Info("Initialize");
}
public Command OnClickReload => new Command(async () =>
{
Info("Reload");
});

public Command OnClickStartExposureNotification => new Command(async () =>
{
UserDialogs.Instance.ShowLoading("Starting ExposureNotification...");
var result = await exposureNotificationService.StartExposureNotification();
var str = $"StartExposureNotification: {result}";
UserDialogs.Instance.HideLoading();
await UserDialogs.Instance.AlertAsync(str, str, Resources.AppResources.ButtonOk);
Copy link
Collaborator

Choose a reason for hiding this comment

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

アラートダイアログのタイトルと内容に同じ文字列を表示する意味はないと思います。
タイトルをStartExposureNotification、内容をResult: True or Falseとしてはいかがでしょうか。

そうれば、ローカル変数strに値を取る必要もないと思います。

Info("StartExposureNotification");
});
public Command OnClickFetchExposureKeyAsync => new Command(async () =>
{
var exLog = "FetchExposureKeyAsync";
Copy link
Collaborator

Choose a reason for hiding this comment

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

exLogの接頭辞exは略さずexceptionと書いた方がわかりやすいです。

また、Logもログを取ることに使っていないので付ける必要はなく、この変数名はシンプルにexceptionで良いと思います。

try { await exposureNotificationService.FetchExposureKeyAsync(); }
catch (Exception ex) { exLog += $":Exception: {ex}"; }
Info(exLog);
});

// see ../Settings/SettingsPageViewModel.cs
public Command OnClickStopExposureNotification => new Command(async () =>
{
UserDialogs.Instance.ShowLoading("Stopping ExposureNotification...");
var result = await exposureNotificationService.StopExposureNotification();
string str = "StopExposureNotification: " + result.ToString();
UserDialogs.Instance.HideLoading();
await UserDialogs.Instance.AlertAsync(str, str, Resources.AppResources.ButtonOk);
Copy link
Collaborator

Choose a reason for hiding this comment

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

ここもOnClickStartExposureNotificationと同様に、アラートダイアログのタイトルと内容に同じ文字列を表示する意味はないと思います。

Info("StopExposureNotification");
});

public Command OnClickRemoveStartDate => new Command(async () =>
{
userDataService.RemoveStartDate();
Info("RemoveStartDate");
});
public Command OnClickRemoveExposureInformation => new Command(async () =>
{
exposureNotificationService.RemoveExposureInformation();
Info("RemoveExposureInformation");
});
public Command OnClickRemoveConfiguration => new Command(async () =>
{
exposureNotificationService.RemoveConfiguration();
Info("RemoveConfiguration");
});
public Command OnClickRemoveLastProcessTekTimestamp => new Command(async () =>
{
exposureNotificationService.RemoveLastProcessTekTimestamp();
Info("RemoveLastProcessTekTimestamp");
});
public Command OnClickRemoveAllUpdateDate => new Command(async () =>
{
termsUpdateService.RemoveAllUpdateDate();
Info("RemoveAllUpdateDate");
});
public Command OnClickQuit => new Command(async () =>
{
Application.Current.Quit();
DependencyService.Get<ICloseApplication>().closeApplication();
});
}
}
Loading