Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Moving UsageService from PullRequestService #2383

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Moving UsageService from PullRequestService #2383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d36c33e
Moving UsageService from PullRequestService to PullRequestCreationVie…
StanleyGoldman Jun 12, 2019
63964ce
Merge branch 'master' into move-usage-service-usage
jcansdale Nov 12, 2019
e09578c
Merge branch 'master' into move-usage-service-usage
jcansdale Mar 15, 2020
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
6 changes: 1 addition & 5 deletions src/GitHub.App/Services/PullRequestService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public class PullRequestService : IssueishService, IPullRequestService, IStaticR
readonly IVSGitExt gitExt;
readonly IGraphQLClientFactory graphqlFactory;
readonly IOperatingSystem os;
readonly IUsageTracker usageTracker;

readonly IDictionary<string, (string commitId, string repoPath)> tempFileMappings;

Expand All @@ -71,16 +70,14 @@ public PullRequestService(
IVSGitExt gitExt,
IApiClientFactory apiClientFactory,
IGraphQLClientFactory graphqlFactory,
IOperatingSystem os,
IUsageTracker usageTracker)
IOperatingSystem os)
: base(apiClientFactory, graphqlFactory)
{
this.gitClient = gitClient;
this.gitService = gitService;
this.gitExt = gitExt;
this.graphqlFactory = graphqlFactory;
this.os = os;
this.usageTracker = usageTracker;
this.tempFileMappings = new Dictionary<string, (string commitId, string repoPath)>(StringComparer.OrdinalIgnoreCase);
}

Expand Down Expand Up @@ -1009,7 +1006,6 @@ async Task<IPullRequestModel> PushAndCreatePR(IModelService modelService,
var ret = await modelService.CreatePullRequest(sourceRepository, targetRepository, sourceBranch, targetBranch, title, body);
await MarkBranchAsPullRequest(repo, sourceBranch.Name, targetRepository.CloneUrl.Owner, ret.Number);
gitExt.RefreshActiveRepositories();
await usageTracker.IncrementCounter(x => x.NumberOfUpstreamPullRequests);
return ret;
}
}
Expand Down
21 changes: 17 additions & 4 deletions src/GitHub.App/ViewModels/GitHubPane/PullRequestCreationViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public class PullRequestCreationViewModel : PanePageViewModelBase, IPullRequestC
readonly IMessageDraftStore draftStore;
readonly IGitService gitService;
readonly IScheduler timerScheduler;
readonly IUsageTracker usageTracker;
readonly CompositeDisposable disposables = new CompositeDisposable();
LocalRepositoryModel activeLocalRepo;
ObservableAsPropertyHelper<RemoteRepositoryModel> githubRepository;
Expand All @@ -53,8 +54,9 @@ public PullRequestCreationViewModel(
INotificationService notifications,
IMessageDraftStore draftStore,
IGitService gitService,
IAutoCompleteAdvisor autoCompleteAdvisor)
: this(modelServiceFactory, service, notifications, draftStore, gitService, autoCompleteAdvisor, DefaultScheduler.Instance)
IAutoCompleteAdvisor autoCompleteAdvisor,
IUsageTracker usageTracker)
: this(modelServiceFactory, service, notifications, draftStore, gitService, autoCompleteAdvisor, usageTracker, DefaultScheduler.Instance)
{
}

Expand All @@ -65,6 +67,7 @@ public PullRequestCreationViewModel(
IMessageDraftStore draftStore,
IGitService gitService,
IAutoCompleteAdvisor autoCompleteAdvisor,
IUsageTracker usageTracker,
IScheduler timerScheduler)
{
Guard.ArgumentNotNull(modelServiceFactory, nameof(modelServiceFactory));
Expand All @@ -81,6 +84,7 @@ public PullRequestCreationViewModel(
this.gitService = gitService;
this.AutoCompleteAdvisor = autoCompleteAdvisor;
this.timerScheduler = timerScheduler;
this.usageTracker = usageTracker;

this.WhenAnyValue(x => x.Branches)
.WhereNotNull()
Expand All @@ -107,8 +111,16 @@ public PullRequestCreationViewModel(
.Subscribe(x => notifications.ShowError(BranchValidator.ValidationResult.Message));

CreatePullRequest = ReactiveCommand.CreateFromObservable(
() => service
.CreatePullRequest(modelService, activeLocalRepo, TargetBranch.Repository, SourceBranch, TargetBranch, PRTitle, Description ?? String.Empty)
() => Observable.FromAsync(async () =>
{
var pullRequestModel = await service
.CreatePullRequest(modelService, activeLocalRepo, TargetBranch.Repository, SourceBranch,
TargetBranch, PRTitle, Description ?? String.Empty);

await usageTracker.IncrementCounter(x => x.NumberOfUpstreamPullRequests);

return pullRequestModel;
})
.Catch<IPullRequestModel, Exception>(ex =>
{
log.Error(ex, "Error creating pull request");
Expand All @@ -120,6 +132,7 @@ public PullRequestCreationViewModel(
return Observable.Empty<IPullRequestModel>();
}),
whenAnyValidationResultChanges);

CreatePullRequest.Subscribe(pr =>
{
notifications.ShowMessage(String.Format(CultureInfo.CurrentCulture, Resources.PRCreatedUpstream, SourceBranch.DisplayName, TargetBranch.Repository.Owner + "/" + TargetBranch.Repository.Name + "#" + pr.Number,
Expand Down
16 changes: 5 additions & 11 deletions test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,8 +578,7 @@ static PullRequestService CreatePullRequestService(Repository repo)
Substitute.For<IVSGitExt>(),
Substitute.For<IApiClientFactory>(),
Substitute.For<IGraphQLClientFactory>(),
serviceProvider.GetOperatingSystem(),
Substitute.For<IUsageTracker>());
serviceProvider.GetOperatingSystem());
return service;
}

Expand Down Expand Up @@ -692,8 +691,7 @@ public void CreatePullRequestAllArgsMandatory()
Substitute.For<IVSGitExt>(),
Substitute.For<IApiClientFactory>(),
Substitute.For<IGraphQLClientFactory>(),
serviceProvider.GetOperatingSystem(),
Substitute.For<IUsageTracker>());
serviceProvider.GetOperatingSystem());

IModelService ms = null;
LocalRepositoryModel sourceRepo = null;
Expand Down Expand Up @@ -906,8 +904,7 @@ public async Task ShouldReturnCorrectDefaultLocalBranchNameForPullRequestsWithNo
Substitute.For<IVSGitExt>(),
Substitute.For<IApiClientFactory>(),
Substitute.For<IGraphQLClientFactory>(),
Substitute.For<IOperatingSystem>(),
Substitute.For<IUsageTracker>());
Substitute.For<IOperatingSystem>());

var localRepo = new LocalRepositoryModel { };
var result = await service.GetDefaultLocalBranchName(localRepo, 123, "コードをレビューする準備ができたこと");
Expand Down Expand Up @@ -1064,25 +1061,22 @@ static PullRequestService CreateTarget(
IVSGitExt gitExt = null,
IApiClientFactory apiClientFactory = null,
IGraphQLClientFactory graphqlFactory = null,
IOperatingSystem os = null,
IUsageTracker usageTracker = null)
IOperatingSystem os = null)
{
gitClient = gitClient ?? Substitute.For<IGitClient>();
gitService = gitService ?? Substitute.For<IGitService>();
gitExt = gitExt ?? Substitute.For<IVSGitExt>();
apiClientFactory = apiClientFactory ?? Substitute.For<IApiClientFactory>();
graphqlFactory = graphqlFactory ?? Substitute.For<IGraphQLClientFactory>();
os = os ?? Substitute.For<IOperatingSystem>();
usageTracker = usageTracker ?? Substitute.For<IUsageTracker>();

return new PullRequestService(
gitClient,
gitService,
gitExt,
apiClientFactory,
graphqlFactory,
os,
usageTracker);
os);
}

static BranchCollection MockBranches(params string[] names)
Expand Down
20 changes: 10 additions & 10 deletions test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestCreationViewModelTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,10 @@ static RemoteRepositoryModel CreateRemoteRepositoryModel(Repository repository)
public async Task TargetBranchDisplayNameIncludesRepoOwnerWhenForkAsync()
{
var data = PrepareTestData("octokit.net", "shana", "master", "octokit", "master", "origin", true, true);
var prservice = new PullRequestService(data.GitClient, data.GitService, Substitute.For<IVSGitExt>(), Substitute.For<IApiClientFactory>(), Substitute.For<IGraphQLClientFactory>(), data.ServiceProvider.GetOperatingSystem(), Substitute.For<IUsageTracker>());
var prservice = new PullRequestService(data.GitClient, data.GitService, Substitute.For<IVSGitExt>(), Substitute.For<IApiClientFactory>(), Substitute.For<IGraphQLClientFactory>(), data.ServiceProvider.GetOperatingSystem());
prservice.GetPullRequestTemplate(data.ActiveRepo).Returns(Observable.Empty<string>());
var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService,
Substitute.For<IMessageDraftStore>(), data.GitService, data.AutoCompleteAdvisor);
Substitute.For<IMessageDraftStore>(), data.GitService, data.AutoCompleteAdvisor, Substitute.For<IUsageTracker>());
await vm.InitializeAsync(data.ActiveRepo, data.Connection);
Assert.That("octokit/master", Is.EqualTo(vm.TargetBranch.DisplayName));
}
Expand Down Expand Up @@ -184,9 +184,9 @@ public async Task CreatingPRsAsync(
var targetBranch = data.TargetBranch;
var ms = data.ModelService;

var prservice = new PullRequestService(data.GitClient, data.GitService, Substitute.For<IVSGitExt>(), Substitute.For<IApiClientFactory>(), Substitute.For<IGraphQLClientFactory>(), data.ServiceProvider.GetOperatingSystem(), Substitute.For<IUsageTracker>());
var prservice = new PullRequestService(data.GitClient, data.GitService, Substitute.For<IVSGitExt>(), Substitute.For<IApiClientFactory>(), Substitute.For<IGraphQLClientFactory>(), data.ServiceProvider.GetOperatingSystem());
var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService,
Substitute.For<IMessageDraftStore>(), data.GitService, data.AutoCompleteAdvisor);
Substitute.For<IMessageDraftStore>(), data.GitService, data.AutoCompleteAdvisor, Substitute.For<IUsageTracker>());
await vm.InitializeAsync(data.ActiveRepo, data.Connection);

// the TargetBranch property gets set to whatever the repo default is (we assume master here),
Expand Down Expand Up @@ -229,7 +229,7 @@ public async Task TemplateIsUsedIfPresentAsync()
prservice.GetPullRequestTemplate(data.ActiveRepo).Returns(Observable.Return("Test PR template"));

var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService,
Substitute.For<IMessageDraftStore>(), data.GitService, data.AutoCompleteAdvisor);
Substitute.For<IMessageDraftStore>(), data.GitService, data.AutoCompleteAdvisor, Substitute.For<IUsageTracker>());
await vm.InitializeAsync(data.ActiveRepo, data.Connection);

Assert.That("Test PR template", Is.EqualTo(vm.Description));
Expand All @@ -249,7 +249,7 @@ public async Task LoadsDraft()

var prservice = Substitute.For<IPullRequestService>();
var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService,
draftStore, data.GitService, data.AutoCompleteAdvisor);
draftStore, data.GitService, data.AutoCompleteAdvisor, Substitute.For<IUsageTracker>());
await vm.InitializeAsync(data.ActiveRepo, data.Connection);

Assert.That(vm.PRTitle, Is.EqualTo("This is a Title."));
Expand All @@ -264,7 +264,7 @@ public async Task UpdatesDraftWhenDescriptionChanges()
var draftStore = Substitute.For<IMessageDraftStore>();
var prservice = Substitute.For<IPullRequestService>();
var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService,
draftStore, data.GitService, data.AutoCompleteAdvisor, scheduler);
draftStore, data.GitService, data.AutoCompleteAdvisor, Substitute.For<IUsageTracker>(), scheduler);
await vm.InitializeAsync(data.ActiveRepo, data.Connection);

vm.Description = "Body changed.";
Expand All @@ -287,7 +287,7 @@ public async Task UpdatesDraftWhenTitleChanges()
var draftStore = Substitute.For<IMessageDraftStore>();
var prservice = Substitute.For<IPullRequestService>();
var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService,
draftStore, data.GitService, data.AutoCompleteAdvisor, scheduler);
draftStore, data.GitService, data.AutoCompleteAdvisor, Substitute.For<IUsageTracker>(), scheduler);
await vm.InitializeAsync(data.ActiveRepo, data.Connection);

vm.PRTitle = "Title changed.";
Expand All @@ -310,7 +310,7 @@ public async Task DeletesDraftWhenPullRequestSubmitted()
var draftStore = Substitute.For<IMessageDraftStore>();
var prservice = Substitute.For<IPullRequestService>();
var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, draftStore,
data.GitService, data.AutoCompleteAdvisor, scheduler);
data.GitService, data.AutoCompleteAdvisor, Substitute.For<IUsageTracker>(), scheduler);
await vm.InitializeAsync(data.ActiveRepo, data.Connection);

await vm.CreatePullRequest.Execute();
Expand All @@ -326,7 +326,7 @@ public async Task DeletesDraftWhenCanceled()
var draftStore = Substitute.For<IMessageDraftStore>();
var prservice = Substitute.For<IPullRequestService>();
var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, draftStore,
data.GitService, data.AutoCompleteAdvisor, scheduler);
data.GitService, data.AutoCompleteAdvisor, Substitute.For<IUsageTracker>(), scheduler);
await vm.InitializeAsync(data.ActiveRepo, data.Connection);

await vm.Cancel.Execute();
Expand Down