Skip to content

Commit 2ef86d9

Browse files
committed
fix: address review comments
1 parent 2544692 commit 2ef86d9

File tree

9 files changed

+78
-25
lines changed

9 files changed

+78
-25
lines changed

packages/preview-middleware-client/src/adp/controllers/AddFragment.controller.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ import { getFragments } from '../api-handler';
2929
import BaseDialog from './BaseDialog.controller';
3030
import { notifyUser } from '../utils';
3131
import { QuickActionTelemetryData } from '../../cpe/quick-actions/quick-action-definition';
32+
import { getFragmentTemplateName } from '../../cpe/additional-change-info/add-xml-additional-info';
3233
import type { AddFragmentData, DeferredXmlFragmentData } from '../add-fragment';
34+
import { setApplicationRequiresReload } from '@sap-ux-private/control-property-editor-common';
35+
import { CommunicationService } from '../../cpe/communication-service';
3336

3437
const radix = 10;
3538

@@ -148,12 +151,17 @@ export default class AddFragment extends BaseDialog<AddFragmentModel> {
148151
targetAggregation: targetAggregation ?? 'content'
149152
};
150153

151-
if(this.data){
154+
if (this.data) {
152155
this.data.deferred.resolve(modifiedValue);
153156
} else {
154157
await this.createFragmentChange(modifiedValue);
155158
}
156159

160+
const templateName = getFragmentTemplateName(this.runtimeControl.getId(), targetAggregation);
161+
if (templateName) {
162+
CommunicationService.sendAction(setApplicationRequiresReload(true));
163+
}
164+
157165
const bundle = await getTextBundle();
158166
notifyUser(bundle.getText('ADP_ADD_FRAGMENT_NOTIFICATION', [fragmentName]), 8000);
159167

packages/preview-middleware-client/src/cpe/additional-change-info/add-xml-additional-info.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export function getAddXMLAdditionalInfo(change: FlexChange<AddXMLChangeContent>)
2626
return undefined;
2727
}
2828

29-
function getFragmentTemplateName(selectorId: string, targetAggregation: string): string {
29+
export function getFragmentTemplateName(selectorId: string, targetAggregation: string): string {
3030
const control = getControlById(selectorId);
3131

3232
if (!control) {

packages/preview-middleware-client/src/flp/enableFakeConnector.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import LrepConnector from 'sap/ui/fl/LrepConnector';
22
import FakeLrepConnector from 'sap/ui/fl/FakeLrepConnector';
3+
import { getAdditionalChangeInfo } from '../utils/additional-change-info';
34

45
import { CHANGES_API_PATH, FlexChange, getFlexSettings } from './common';
56

@@ -37,6 +38,8 @@ export async function create(changes: FlexChange | FlexChange[]): Promise<void>
3738
change.support.generator = settings.generator;
3839
}
3940

41+
const additionalChangeInfo = getAdditionalChangeInfo(change);
42+
4043
if (typeof FakeLrepConnector.fileChangeRequestNotifier === 'function' && change.fileName) {
4144
try {
4245
FakeLrepConnector.fileChangeRequestNotifier(change.fileName, 'create', change);
@@ -45,9 +48,15 @@ export async function create(changes: FlexChange | FlexChange[]): Promise<void>
4548
}
4649
}
4750

51+
const body = {
52+
change,
53+
additionalChangeInfo
54+
};
55+
56+
4857
return fetch(CHANGES_API_PATH, {
4958
method: 'POST',
50-
body: JSON.stringify(change, null, 2),
59+
body: JSON.stringify(body, null, 2),
5160
headers: {
5261
'content-type': 'application/json'
5362
}
Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
export default class AddXMLPlugin {
2-
constructor() {}
3-
public execute = jest.fn();
4-
public add = jest.fn();
5-
}
1+
const AddXMLPlugin = jest.fn().mockImplementation(() => {
2+
return {
3+
execute: jest.fn(),
4+
add: jest.fn()
5+
};
6+
});
7+
8+
export default AddXMLPlugin;

packages/preview-middleware-client/test/unit/adp/add-fragment.test.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
import { initAddXMLPlugin } from '../../../src/adp/add-fragment';
22
import type RuntimeAuthoring from 'sap/ui/rta/RuntimeAuthoring';
33
import type UI5Element from 'sap/ui/core/Element';
4-
import CommandFactory from 'sap/ui/rta/command/CommandFactory';
5-
import AddXMLPlugin from 'sap/ui/rta/plugin/AddXMLPlugin';
4+
import CommandFactory from 'mock/sap/ui/rta/command/CommandFactory';
5+
import AddXMLPlugin from 'mock/sap/ui/rta/plugin/AddXMLPlugin';
66
import { DialogFactory, DialogNames } from '../../../src/adp/dialog-factory';
77
import { createDeferred } from '../../../src/adp/utils';
88

9-
jest.mock('sap/ui/rta/command/CommandFactory');
10-
jest.mock('sap/ui/rta/plugin/AddXMLPlugin');
119
jest.mock('../../../src/adp/dialog-factory');
1210
jest.mock('../../../src/adp/utils', () => ({
1311
createDeferred: jest.fn()
@@ -31,13 +29,10 @@ describe('AddFragmentService', () => {
3129

3230
describe('init', () => {
3331
it('should initialize the AddXMLPlugin and set it in RTA plugins', async () => {
34-
const mockCommandFactory = jest.fn();
35-
(CommandFactory as unknown as jest.Mock).mockImplementation(() => mockCommandFactory);
36-
3732
initAddXMLPlugin(mockRta);
3833

3934
expect(AddXMLPlugin).toHaveBeenCalledWith({
40-
commandFactory: mockCommandFactory,
35+
commandFactory: expect.any(CommandFactory),
4136
fragmentHandler: expect.any(Function)
4237
});
4338

packages/preview-middleware-client/test/unit/adp/controllers/AddFragment.controller.test.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@ import type ManagedObject from 'sap/ui/base/ManagedObject';
1717
import Core from 'sap/ui/core/Core';
1818
import { type AddFragmentChangeContentType } from 'sap/ui/fl/Change';
1919
import { AddFragmentData } from '../../../../src/adp/add-fragment';
20+
import * as addXMLAdditionalInfo from '../../../../src/cpe/additional-change-info/add-xml-additional-info';
21+
import { CommunicationService } from '../../../../src/cpe/communication-service';
22+
23+
jest.mock('@sap-ux-private/control-property-editor-common');
24+
jest.mock('../../../../src/cpe/communication-service', () => ({
25+
CommunicationService: {
26+
sendAction: jest.fn()
27+
}
28+
}));
2029

2130
describe('AddFragment', () => {
2231
beforeAll(() => {
@@ -624,6 +633,7 @@ describe('AddFragment', () => {
624633
const rtaMock = new RuntimeAuthoringMock({} as RTAOptions);
625634

626635
test('creates new fragment and a change', async () => {
636+
const mockSendAction = CommunicationService.sendAction as jest.Mock;
627637
sapMock.ui.version = '1.71.62';
628638
const executeSpy = jest.fn();
629639
rtaMock.getCommandStack.mockReturnValue({
@@ -641,8 +651,10 @@ describe('AddFragment', () => {
641651
getAllAggregations: jest.fn().mockReturnValue({}),
642652
getName: jest.fn().mockReturnValue('sap.uxap.ObjectPageLayout'),
643653
getDefaultAggregationName: jest.fn().mockReturnValue('content')
644-
})
654+
}),
655+
getId: jest.fn().mockReturnValue('some-id')
645656
} as unknown as ManagedObject);
657+
jest.spyOn(addXMLAdditionalInfo, 'getFragmentTemplateName').mockReturnValue('templateName');
646658

647659
const addFragment = new AddFragment(
648660
'adp.extension.controllers.AddFragment',
@@ -700,6 +712,7 @@ describe('AddFragment', () => {
700712
}
701713
});
702714
expect(CommandFactory.getCommandFor.mock.calls[0][4].selector).toBeUndefined();
715+
expect(mockSendAction).toHaveBeenCalled();
703716
});
704717

705718
test('add header field fragment and a change if targetAggregation is items and not a dynamic header', async () => {
@@ -783,8 +796,10 @@ describe('AddFragment', () => {
783796
getMetadata: jest.fn().mockReturnValue({
784797
getName: jest.fn().mockReturnValue('No.Dynamic.ObjectPageDynamicHeaderContent')
785798
})
786-
})
799+
}),
800+
getId: jest.fn().mockReturnValue('some-id')
787801
} as unknown as ManagedObject);
802+
jest.spyOn(addXMLAdditionalInfo, 'getFragmentTemplateName').mockReturnValue('');
788803

789804
addFragment.handleDialogClose = jest.fn();
790805

@@ -890,8 +905,10 @@ describe('AddFragment', () => {
890905
getMetadata: jest.fn().mockReturnValue({
891906
getName: jest.fn().mockReturnValue('sap.uxap.ObjectPageLayout')
892907
})
893-
})
908+
}),
909+
getId: jest.fn().mockReturnValue('some-id')
894910
} as unknown as ManagedObject);
911+
jest.spyOn(addXMLAdditionalInfo, 'getFragmentTemplateName').mockReturnValue('');
895912

896913
addFragment.handleDialogClose = jest.fn();
897914

@@ -931,8 +948,10 @@ describe('AddFragment', () => {
931948
getAllAggregations: jest.fn().mockReturnValue({}),
932949
getName: jest.fn().mockReturnValue('sap.uxap.ObjectPageLayout'),
933950
getDefaultAggregationName: jest.fn().mockReturnValue('content')
934-
})
951+
}),
952+
getId: jest.fn().mockReturnValue('some-id')
935953
} as unknown as ManagedObject);
954+
jest.spyOn(addXMLAdditionalInfo, 'getFragmentTemplateName').mockReturnValue('templateName');
936955
const event = {
937956
getSource: jest.fn().mockReturnValue({
938957
setEnabled: jest.fn()
@@ -972,7 +991,7 @@ describe('AddFragment', () => {
972991
fragment: `<core:FragmentDefinition xmlns:core='sap.ui.core'></core:FragmentDefinition>`,
973992
fragmentPath: `fragments/test.fragment.xml`,
974993
index: 0,
975-
targetAggregation: 'content',
994+
targetAggregation: 'content'
976995
});
977996
});
978997
});

packages/preview-middleware-client/test/unit/flp/WorkspaceConnector.test.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import ObjectStorageConnector from 'mock/sap/ui/fl/write/api/connectors/ObjectStorageConnector';
22
import connector from '../../../src/flp/WorkspaceConnector';
33
import VersionInfo from 'mock/sap/ui/VersionInfo';
4+
import * as additionalChangeInfo from '../../../src/utils/additional-change-info';
45

56
import { documentMock, fetchMock } from 'mock/window';
67

78
describe('flp/WorkspaceConnector', () => {
9+
jest.spyOn(additionalChangeInfo, 'getAdditionalChangeInfo').mockReturnValue(undefined);
810
test('layers', () => {
911
expect(connector.layers).toEqual(['VENDOR', 'CUSTOMER_BASE']);
1012
});
@@ -15,15 +17,25 @@ describe('flp/WorkspaceConnector', () => {
1517
});
1618

1719
test('setItem', async () => {
20+
jest.spyOn(additionalChangeInfo, 'getAdditionalChangeInfo').mockReturnValueOnce({
21+
templateName: 'templateName'
22+
});
1823
connector.storage.fileChangeRequestNotifier = jest.fn();
1924
const change = { data: '~Data' };
2025
await connector.storage.setItem('~notUsed', change);
2126

27+
const body = {
28+
change: {
29+
...change
30+
},
31+
additionalChangeInfo: { templateName: 'templateName' }
32+
};
33+
2234
expect(fetch).toHaveBeenCalledWith(
2335
expect.anything(),
2436
expect.objectContaining({
2537
method: 'POST',
26-
body: JSON.stringify({change: {...change}}, null, 2)
38+
body: JSON.stringify(body, null, 2)
2739
})
2840
);
2941
expect(connector.storage.fileChangeRequestNotifier).toHaveBeenCalledTimes(0);
@@ -38,10 +50,15 @@ describe('flp/WorkspaceConnector', () => {
3850
expect.anything(),
3951
expect.objectContaining({
4052
method: 'POST',
41-
body: JSON.stringify({change: {...change}}, null, 2)
53+
body: JSON.stringify({ change: { ...change } }, null, 2)
4254
})
4355
);
44-
expect(connector.storage.fileChangeRequestNotifier).toHaveBeenCalledWith('dummyFile', 'create', change, undefined);
56+
expect(connector.storage.fileChangeRequestNotifier).toHaveBeenCalledWith(
57+
'dummyFile',
58+
'create',
59+
change,
60+
undefined
61+
);
4562
});
4663

4764
test('setItem, generator - tool-variant', async () => {

packages/preview-middleware-client/test/unit/flp/enableFakeConnector.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ import FakeLrepConnector from 'mock/sap/ui/fl/FakeLrepConnector';
33

44
import enableFakeConnector, { loadChanges, create } from '../../../src/flp/enableFakeConnector';
55
import LrepConnector from 'mock/sap/ui/fl/LrepConnector';
6+
import * as additionalChangeInfo from '../../../src/utils/additional-change-info';
67

78
describe('flp/FakeLrepConnector', () => {
9+
jest.spyOn(additionalChangeInfo, 'getAdditionalChangeInfo').mockReturnValue(undefined);
810
afterEach(() => {
911
jest.restoreAllMocks();
1012
});

packages/preview-middleware-client/tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"extends": "../../tsconfig.json",
33
"include": [
4-
"src/**/*.ts",
4+
"src",
55
"test",
66
"test/**/*.json",
77
"types"

0 commit comments

Comments
 (0)