Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ public async Task<ActionResult<IEnumerable<CommentItemModel>>> GetCommentsAsync(
public async Task<ActionResult<IEnumerable<CommentItemModel>>> GetConversationInfoAsync(string reviewId, string apiRevisionId)
{
var comments = await _commentsManager.GetCommentsAsync(reviewId, false);
var commentsInAPIRevision = comments.Where(c => c.APIRevisionId == apiRevisionId).ToList();
var commentsInAPIRevision = comments.Where(c => c.CommentType == CommentType.APIRevision).ToList();
var sampleComments = comments.Where(c => c.CommentType == CommentType.SampleRevision).ToList();

var totalActiveConversiations = 0;
var totalActiveConversationInApiRevision = 0;
var totalActiveConversationInSampleRevision = 0;
var totalActiveConversationInApiRevisions = 0;
var totalActiveConversationInSampleRevisions = 0;

foreach (var group in comments.GroupBy(c => c.ElementId))
{
Expand All @@ -70,22 +70,22 @@ public async Task<ActionResult<IEnumerable<CommentItemModel>>> GetConversationIn
{
if (!group.Any(c => c.IsResolved))
{
totalActiveConversationInSampleRevision++;
totalActiveConversationInApiRevisions++;
}
}

foreach (var group in commentsInAPIRevision.GroupBy(c => c.ElementId))
{
if (!group.Any(c => c.IsResolved))
{
totalActiveConversationInApiRevision++;
totalActiveConversationInSampleRevisions++;
}
}

dynamic conversationInfobject = new ExpandoObject();
conversationInfobject.TotalActiveConversations = totalActiveConversiations;
conversationInfobject.ActiveConversationsInActiveAPIRevision = totalActiveConversationInApiRevision;
conversationInfobject.ActiveConversationsInSampleRevisions = totalActiveConversationInSampleRevision;
conversationInfobject.ActiveConversationsInActiveAPIRevision = totalActiveConversationInApiRevisions;
conversationInfobject.ActiveConversationsInSampleRevisions = totalActiveConversationInSampleRevisions;
return new LeanJsonResult(conversationInfobject, StatusCodes.Status200OK);
}

Expand Down
10 changes: 9 additions & 1 deletion src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,15 @@ public async Task CleanupPullRequestData()

private async Task<IssueComment> GetExistingCommentForPackage(string repoOwner, string repoName, int pr)
{
var comments = await _githubClient.Issue.Comment.GetAllForIssue(repoOwner, repoName, pr);
IReadOnlyList<IssueComment> comments = null;
try
{
comments = await _githubClient.Issue.Comment.GetAllForIssue(repoOwner, repoName, pr);
}
catch (Exception ex)
{
_telemetryClient.TrackException(ex);
}
if (comments != null)
{
// Check for comment created for current package.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ChangeDetectorRef, Component, ElementRef, Input, OnChanges, OnInit, SimpleChanges, ViewChild, ViewContainerRef } from '@angular/core';
import { ChangeDetectorRef, Component, ElementRef, EventEmitter, Input, OnChanges, OnInit, Output, SimpleChanges, ViewChild, ViewContainerRef } from '@angular/core';
import { take } from 'rxjs/operators';
import { CommentItemModel, CommentType } from 'src/app/_models/review';
import { CodePanelData, CodePanelRowDatatype, StructuredToken } from 'src/app/_models/revision';
Expand Down Expand Up @@ -29,6 +29,8 @@ export class CodePanelComponent implements OnChanges{
@Input() showLineNumbers: boolean = true;
@Input() loadFailed : boolean = false;

@Output() hasActiveConversation : EventEmitter<boolean> = new EventEmitter<boolean>();

isLoading: boolean = true;
codeWindowHeight: string | undefined = undefined;
codePanelRowDataIndicesMap = new Map<string, number>();
Expand All @@ -47,6 +49,7 @@ export class CodePanelComponent implements OnChanges{
if (changes['codePanelRowData']) {
if (changes['codePanelRowData'].currentValue.length > 0) {
this.loadCodePanelViewPort();
this.updateHasActiveConversations();
} else {
this.isLoading = true;
this.codePanelRowSource = undefined;
Expand Down Expand Up @@ -436,6 +439,7 @@ export class CodePanelComponent implements OnChanges{
next: () => {
this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup].comments.filter(c => c.id === data.commentId)[0].commentText = data.commentText;
this.updateItemInScroller(this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup]);
this.updateHasActiveConversations();
}
});
}
Expand All @@ -447,6 +451,7 @@ export class CodePanelComponent implements OnChanges{
comments.push(response);
this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup].comments = [...comments]
this.updateItemInScroller(this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup]);
this.updateHasActiveConversations();
}
}
);
Expand All @@ -465,6 +470,7 @@ export class CodePanelComponent implements OnChanges{
} else {
this.updateItemInScroller(this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup]);
}
this.updateHasActiveConversations();
}
});
}
Expand All @@ -476,6 +482,7 @@ export class CodePanelComponent implements OnChanges{
this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup].isResolvedCommentThread = true;
this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup].commentThreadIsResolvedBy = this.userProfile?.userName!;
this.updateItemInScroller({ ...this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup]});
this.updateHasActiveConversations();
}
});
}
Expand All @@ -485,6 +492,7 @@ export class CodePanelComponent implements OnChanges{
this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup].isResolvedCommentThread = false;
this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup].commentThreadIsResolvedBy = '';
this.updateItemInScroller({ ...this.codePanelData!.nodeMetaData[data.nodeIdHashed!].commentThread[data.associatedRowPositionInGroup]});
this.updateHasActiveConversations();
}
});
}
Expand All @@ -506,6 +514,19 @@ export class CodePanelComponent implements OnChanges{
});
}

private updateHasActiveConversations() {
let hasActiveConversations = false;
for (let row of this.codePanelRowData) {
if (row.type === CodePanelRowDatatype.CommentThread) {
if (row.comments && row.comments.length > 0 && row.isResolvedCommentThread === false) {
hasActiveConversations = true;
break;
}
}
}
this.hasActiveConversation.emit(hasActiveConversations);
}

private loadCodePanelViewPort() {
this.setMaxLineNumberWidth();
this.initializeDataSource().then(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@
[modal]="true" [(visible)]="showAPIRevisionApprovalModal"
[style]="{ width: '40dvw' }">
<ul class="list-group">
<li class="list-group-item" *ngIf="this.conversiationInfo && this.conversiationInfo.totalActiveConversationInApiRevision > 0 && this.conversiationInfo.totalActiveConversationInSampleRevision > 0">
<li class="list-group-item" *ngIf="hasActiveConversation">
<div class="float-start">
<p class="mb-0">There are still active conversations. Would you like to review these first?</p>
</div>
Expand All @@ -173,6 +173,6 @@
<div class="d-grid gap-2 mt-2">
<button class="btn btn-success" type="button"
(click)="toggleAPIRevisionApproval()"
[disabled]="(!hasActiveConversation || (hasActiveConversation && overrideActiveConversationforApproval)) && (!hasFatalDiagnostics ||(hasFatalDiagnostics && overrideFatalDiagnosticsforApproval))">Confirm</button>
[disabled]="(hasActiveConversation && !overrideActiveConversationforApproval) || (hasFatalDiagnostics && !overrideFatalDiagnosticsforApproval)">Confirm</button>
</div>
</p-dialog>
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
@Input() activeAPIRevision : APIRevision | undefined = undefined;
@Input() diffAPIRevision : APIRevision | undefined = undefined;
@Input() preferedApprovers: string[] = [];
@Input() conversiationInfo : any | undefined = undefined;
@Input() hasFatalDiagnostics : boolean = false;
@Input() hasActiveConversation : boolean = false;
@Input() hasHiddenAPIs : boolean = false;

@Output() diffStyleEmitter : EventEmitter<string> = new EventEmitter<string>();
Expand Down Expand Up @@ -51,7 +51,6 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
apiRevisionApprovalMessage: string = '';
apiRevisionApprovalBtnClass: string = '';
apiRevisionApprovalBtnLabel: string = '';
hasActiveConversation : string = '';
showAPIRevisionApprovalModal: boolean = false;
overrideActiveConversationforApproval : boolean = false;
overrideFatalDiagnosticsforApproval : boolean = false;
Expand Down Expand Up @@ -97,7 +96,6 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
this.showLineNumbersSwitch = true;
}

this.setHasActiveConversatons();
this.setAPIRevisionApprovalStates();
this.setReviewApprovalStatus();
}
Expand Down Expand Up @@ -236,10 +234,6 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
}
}

setHasActiveConversatons() {
this.hasActiveConversation = this.conversiationInfo && this.conversiationInfo.totalActiveConversationInApiRevision > 0 && this.conversiationInfo.totalActiveConversationInSampleRevision > 0;
}

handleAPIRevisionApprovalAction() {
if (!this.activeAPIRevisionIsApprovedByCurrentUser && (this.hasActiveConversation || this.hasFatalDiagnostics)) {
this.showAPIRevisionApprovalModal = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@
[userProfile]="userProfile"
[loadFailed]="loadFailed"
[showLineNumbers]="showLineNumbers" [scrollToNodeIdHashed]="scrollToNodeIdHashed"
[scrollToNodeId]="scrollToNodeId"></app-code-panel>
[scrollToNodeId]="scrollToNodeId"
(hasActiveConversation)="handleHasActiveConversationEmitter($event)"></app-code-panel>
</div>
</ng-template>
<ng-template pTemplate>
Expand All @@ -47,8 +48,8 @@
[activeAPIRevision]="activeAPIRevision"
[diffAPIRevision]="diffAPIRevision"
[preferedApprovers]="preferedApprovers"
[conversiationInfo]="conversiationInfo"
[hasFatalDiagnostics]="hasFatalDiagnostics"
[hasActiveConversation]="hasActiveConversation"
[hasHiddenAPIs]="hasHiddenAPIs"
(showSystemCommentsEmitter)="handleShowSystemCommentsEmitter($event)"
(showDocumentationEmitter)="handleShowDocumentationEmitter($event)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ export class ReviewPageComponent implements OnInit {
scrollToNodeId : string | undefined = undefined;
showLineNumbers : boolean = true;
preferedApprovers : string[] = [];
conversiationInfo : any | undefined = undefined;
hasFatalDiagnostics : boolean = false;
hasActiveConversation : boolean = false;
hasHiddenAPIs : boolean = false;
loadFailed : boolean = false;

Expand Down Expand Up @@ -128,7 +128,6 @@ export class ReviewPageComponent implements OnInit {
this.workerService.startWorker().then(() => {
this.registerWorkerEventHandler();
this.loadReviewContent(this.reviewId!, this.activeApiRevisionId, this.diffApiRevisionId);
this.loadConversationInfo(this.reviewId!, this.activeApiRevisionId!);
});
}

Expand Down Expand Up @@ -197,15 +196,6 @@ export class ReviewPageComponent implements OnInit {
});
}

loadConversationInfo(reviewId: string, apiRevisionId: string) {
this.commentsService.getConversationInfo(reviewId, apiRevisionId)
.pipe(takeUntil(this.destroy$)).subscribe({
next: (response: any) => {
this.conversiationInfo = response;
}
});
}

loadAPIRevisions(noOfItemsRead : number, pageSize: number) {
// Ensure existing subscription is destroyed
this.destroyLoadAPIRevision$?.next();
Expand Down Expand Up @@ -377,6 +367,8 @@ export class ReviewPageComponent implements OnInit {
this.apiRevisionsService.toggleAPIRevisionViewedByForUser(this.activeApiRevisionId!, state).pipe(take(1)).subscribe({
next: (apiRevision: APIRevision) => {
this.activeAPIRevision = apiRevision;
const activeAPIRevisionIndex = this.apiRevisions.findIndex(x => x.id === this.activeAPIRevision!.id);
this.apiRevisions[activeAPIRevisionIndex] = this.activeAPIRevision!;
}
});
}
Expand All @@ -386,6 +378,8 @@ export class ReviewPageComponent implements OnInit {
this.apiRevisionsService.toggleAPIRevisionApproval(this.reviewId!, this.activeApiRevisionId!).pipe(take(1)).subscribe({
next: (apiRevision: APIRevision) => {
this.activeAPIRevision = apiRevision;
const activeAPIRevisionIndex = this.apiRevisions.findIndex(x => x.id === this.activeAPIRevision!.id);
this.apiRevisions[activeAPIRevisionIndex] = this.activeAPIRevision!;
}
});
}
Expand All @@ -412,6 +406,10 @@ export class ReviewPageComponent implements OnInit {
});
}

handleHasActiveConversationEmitter(value: boolean) {
this.hasActiveConversation = value;
}

checkForFatalDiagnostics() {
for (const rowData of this.codePanelRowData) {
if (rowData.diagnostics && rowData.diagnostics.level === 'fatal') {
Expand Down
21 changes: 17 additions & 4 deletions src/dotnet/APIView/apiview.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ extends:
value: "DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://127.0.0.1:10000/devstoreaccount1"
- name: CosmosEmulatorConnectionString
value: "AccountEndpoint=https://localhost:8081/;AccountKey=C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw=="
- name: AzureSdkForNetDevOpsFeed
value: "https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-net/nuget/v3/index.json"
- name: CSharpAPIParserVersion
value: "1.0.0-dev.20240626.4"
- name: TestingDataContainer
value: "https://apiviewuitest.blob.core.windows.net/testingdata"
- ${{ if ne(variables['System.TeamProject'], 'internal') }}:
Expand Down Expand Up @@ -410,6 +406,19 @@ extends:
azureSubscription: 'APIViewUI-Instance-deployment-connection'
WebAppName: apiviewuat
packageForLinux: $(Pipeline.Workspace)/APIView/**/*.zip
InlineScript: |
@echo off
echo Installing npm packages
call npm install --prefix D:\home\site\wwwroot --force
call npm run-script build --prefix D:\home\site\wwwroot --force
echo Installing .NET tool
call mkdir D:\home\site\wwwroot\csharp-parser
call dotnet tool install --add-source ${{ parameters.AzureSdkForNetDevOpsFeed }} --version ${{ parameters.CSharpAPIParserVersion }} --tool-path D:\home\site\wwwroot\csharp-parser CSharpAPIParser
echo Installing Python tools
call D:\home\site\wwwroot\Python\python -m pip uninstall api-stub-generator
call D:\home\site\wwwroot\Python\python -m pip install pylint==2.13.9 pylint-guidelines-checker==0.0.6 api-stub-generator==0.3.2 --index-url "https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-python/pypi/simple/"
enableCustomDeployment: true
DeploymentType: zipDeploy

- stage: Production_Publish
displayName: Prod
Expand Down Expand Up @@ -463,6 +472,10 @@ extends:
echo Installing npm packages
call npm install --prefix D:\home\site\wwwroot --force
call npm run-script build --prefix D:\home\site\wwwroot --force
echo Installing .NET tool
call mkdir D:\home\site\wwwroot\csharp-parser
call dotnet tool install --add-source ${{ parameters.AzureSdkForNetDevOpsFeed }} --version ${{ parameters.CSharpAPIParserVersion }} --tool-path D:\home\site\wwwroot\csharp-parser CSharpAPIParser
echo Installing Python tools
call D:\home\site\wwwroot\Python\python -m pip uninstall api-stub-generator
call D:\home\site\wwwroot\Python\python -m pip install pylint==2.13.9 pylint-guidelines-checker==0.0.6 api-stub-generator==0.3.3 --index-url "https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-python/pypi/simple/"
enableCustomDeployment: true
Expand Down