Skip to content

Commit

Permalink
NAS-133365 / 25.04 / Reporting exporters form is broken (#11282)
Browse files Browse the repository at this point in the history
  • Loading branch information
undsoft authored Jan 6, 2025
1 parent 2df646d commit 9e50429
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 70 deletions.
4 changes: 2 additions & 2 deletions src/app/interfaces/api/api-call-directory.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ import {
ReplicationTask,
} from 'app/interfaces/replication-task.interface';
import {
CreateReportingExporter, ReportingExporter, ReportingExporterSchema, UpdateReportingExporter,
ReportingExporter, ReportingExporterSchema, UpdateReportingExporter,
} from 'app/interfaces/reporting-exporters.interface';
import { ReportingGraph } from 'app/interfaces/reporting-graph.interface';
import {
Expand Down Expand Up @@ -695,7 +695,7 @@ export interface ApiCallDirectory {
'replication.update': { params: [id: number, update: Partial<ReplicationCreate>]; response: ReplicationTask };

// Reporting
'reporting.exporters.create': { params: [CreateReportingExporter]; response: ReportingExporter };
'reporting.exporters.create': { params: [UpdateReportingExporter]; response: ReportingExporter };
'reporting.exporters.delete': { params: [id: number]; response: boolean };
'reporting.exporters.exporter_schemas': { params: void; response: ReportingExporterSchema[] };
'reporting.exporters.query': { params: QueryParams<ReportingExporter>; response: ReportingExporter[] };
Expand Down
4 changes: 1 addition & 3 deletions src/app/interfaces/reporting-exporters.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ export interface ReportingExporterList {
export interface ReportingExporter {
name: string;
id: number;
type: string;
enabled: boolean;
attributes: Record<string, unknown>;
}

export type CreateReportingExporter = Omit<ReportingExporter, 'id'>;
export type UpdateReportingExporter = Omit<ReportingExporter, 'id' | 'type'>;
export type UpdateReportingExporter = Partial<Omit<ReportingExporter, 'id'>>;
2 changes: 1 addition & 1 deletion src/app/interfaces/schema.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface OldSchema {
type: SchemaType | SchemaType[];
_name_: string;
_required_: boolean;

const?: string;
}

export interface SchemaProperties {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ describe('ReportingExportersFormComponent', () => {
const existingExporter: ReportingExporter = {
name: 'test',
id: 123,
type: ReportingExporterKey.Graphite,
attributes: {
exporter_type: ReportingExporterKey.Graphite,
access_key_id: 'access_key_id',
secret_access_key: 'secret_access_key',
},
Expand Down Expand Up @@ -94,11 +94,11 @@ describe('ReportingExportersFormComponent', () => {

expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('reporting.exporters.create', [{
name: 'exporter1',
type: ReportingExporterKey.Graphite,
enabled: true,
attributes: {
access_key_id: 'abcde',
secret_access_key: 'abcd',
exporter_type: ReportingExporterKey.Graphite,
},
}]);
expect(spectator.inject(SlideInRef).close).toHaveBeenCalled();
Expand Down Expand Up @@ -130,7 +130,7 @@ describe('ReportingExportersFormComponent', () => {

expect(values).toEqual({
Name: existingExporter.name,
Type: existingExporter.type,
Type: existingExporter.attributes.exporter_type,
Enable: existingExporter.enabled,
'Secret Access Key ID': existingExporter.attributes.secret_access_key,
'Access Key ID': existingExporter.attributes.access_key_id,
Expand Down Expand Up @@ -163,6 +163,7 @@ describe('ReportingExportersFormComponent', () => {
attributes: {
secret_access_key: existingExporter.attributes.secret_access_key,
access_key_id: 'efghi',
exporter_type: ReportingExporterKey.Graphite,
},
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,10 @@ export class ReportingExportersFormComponent implements OnInit {
this.createExporterControls(schemas);

if (!this.isNew) {
this.form.patchValue(this.editingExporter);
this.form.patchValue({
...this.editingExporter,
type: this.editingExporter.attributes['exporter_type'] as string,
});
}

this.isLoading = false;
Expand Down Expand Up @@ -162,7 +165,7 @@ export class ReportingExportersFormComponent implements OnInit {
for (const input of schema.schema) {
this.form.controls.attributes.addControl(
input._name_,
new FormControl('', input._required_ ? [Validators.required] : []),
new FormControl(input.const || '', input._required_ ? [Validators.required] : []),
);
}
}
Expand All @@ -180,7 +183,9 @@ export class ReportingExportersFormComponent implements OnInit {
}

parseSchemaForDynamicSchema(schema: ReportingExporterSchema): DynamicFormSchemaNode[] {
return schema.schema.map((input) => getDynamicFormSchemaNode(input));
return schema.schema
.filter((input) => !input.const)
.map((input) => getDynamicFormSchemaNode(input));
}

parseSchemaForExporterList(schema: ReportingExporterSchema): ReportingExporterList {
Expand Down Expand Up @@ -217,9 +222,8 @@ export class ReportingExportersFormComponent implements OnInit {
...this.form.value,
};

if (!this.isNew) {
delete values.type;
}
values.attributes['exporter_type'] = values.type;
delete values.type;

for (const [key, value] of Object.entries(values.attributes)) {
if (value == null || value === '') {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { HarnessLoader } from '@angular/cdk/testing';
import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
import { MatButtonHarness } from '@angular/material/button/testing';
import { MatSlideToggleHarness } from '@angular/material/slide-toggle/testing';
import { Spectator, createComponentFactory, mockProvider } from '@ngneat/spectator/jest';
import { of } from 'rxjs';
import { mockCall, mockApi } from 'app/core/testing/utils/mock-api.utils';
Expand All @@ -23,10 +24,10 @@ const exporters: ReportingExporter[] = [
attributes: {
secret: 'abcd',
email: 'testemail',
exporter_type: ReportingExporterKey.Graphite,
},
enabled: true,
name: 'test',
type: ReportingExporterKey.Graphite,
},
];

Expand Down Expand Up @@ -98,6 +99,16 @@ describe('ReportingExportersListComponent', () => {
expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('reporting.exporters.delete', [1]);
});

it('updates a reporting exporter when Enabled checkbox is toggled', async () => {
const toggle = await table.getHarnessInCell(MatSlideToggleHarness, 1, 2);
await toggle.toggle();

expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('reporting.exporters.update', [
1,
{ enabled: false },
]);
});

it('should show table rows', async () => {
const expectedRows = [
['Name', 'Type', 'Enabled', ''],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class ReportingExporterListComponent implements OnInit {
}),
textColumn({
title: this.translate.instant('Type'),
propertyName: 'type',
getValue: (row) => row.attributes['exporter_type'],
}),
toggleColumn({
title: this.translate.instant('Enabled'),
Expand All @@ -93,13 +93,13 @@ export class ReportingExporterListComponent implements OnInit {
{ name: row.name, checked: checked ? 'Enabling' : 'Disabling' },
),
);
const exporter = { ...row };
delete exporter.type;
delete exporter.id;
this.api.call('reporting.exporters.update', [row.id, { ...exporter, enabled: checked }]).pipe(
this.api.call('reporting.exporters.update', [row.id, { enabled: checked }]).pipe(
untilDestroyed(this),
).subscribe({
complete: () => this.appLoader.close(),
complete: () => {
this.appLoader.close();
this.getExporters();
},
error: (error: unknown) => this.errorCaught(error),
});
},
Expand Down
Loading

0 comments on commit 9e50429

Please sign in to comment.