Skip to content

Commit c2b14fb

Browse files
JakobVogelsangJakobVogelsang
authored
feat(Editing): check globally for ID uniqueness
1 parent a3c1368 commit c2b14fb

File tree

5 files changed

+197
-11
lines changed

5 files changed

+197
-11
lines changed

‎src/Editing.ts‎

Lines changed: 100 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export function Editing<TBase extends LitElementConstructor>(Base: TBase) {
5050
)
5151
return true;
5252

53-
const invalid =
53+
const invalidNaming =
5454
create.new.element.hasAttribute('name') &&
5555
Array.from(create.new.parent.children).some(
5656
elm =>
@@ -59,7 +59,7 @@ export function Editing<TBase extends LitElementConstructor>(Base: TBase) {
5959
(<Element>create.new.element).getAttribute('name')
6060
);
6161

62-
if (invalid)
62+
if (invalidNaming) {
6363
this.dispatchEvent(
6464
newLogEvent({
6565
kind: 'error',
@@ -77,7 +77,38 @@ export function Editing<TBase extends LitElementConstructor>(Base: TBase) {
7777
})
7878
);
7979

80-
return !invalid;
80+
return false;
81+
}
82+
83+
const invalidId =
84+
create.new.element.hasAttribute('id') &&
85+
Array.from(
86+
create.new.parent.ownerDocument.querySelectorAll(
87+
'LNodeType, DOType, DAType, EnumType'
88+
)
89+
).some(
90+
elm =>
91+
elm.getAttribute('id') ===
92+
(<Element>create.new.element).getAttribute('id')
93+
);
94+
95+
if (invalidId) {
96+
this.dispatchEvent(
97+
newLogEvent({
98+
kind: 'error',
99+
title: get('editing.error.create', {
100+
name: create.new.element.tagName,
101+
}),
102+
message: get('editing.error.idClash', {
103+
id: create.new.element.getAttribute('id')!,
104+
}),
105+
})
106+
);
107+
108+
return false;
109+
}
110+
111+
return true;
81112
}
82113

83114
private onCreate(action: Create) {
@@ -199,7 +230,7 @@ export function Editing<TBase extends LitElementConstructor>(Base: TBase) {
199230
private checkReplaceValidity(replace: Replace): boolean {
200231
if (replace.checkValidity !== undefined) return replace.checkValidity();
201232

202-
const invalid =
233+
const invalidNaming =
203234
replace.new.element.hasAttribute('name') &&
204235
replace.new.element.getAttribute('name') !==
205236
replace.old.element.getAttribute('name') &&
@@ -210,7 +241,7 @@ export function Editing<TBase extends LitElementConstructor>(Base: TBase) {
210241
replace.new.element.getAttribute('name')
211242
);
212243

213-
if (invalid)
244+
if (invalidNaming) {
214245
this.dispatchEvent(
215246
newLogEvent({
216247
kind: 'error',
@@ -225,7 +256,40 @@ export function Editing<TBase extends LitElementConstructor>(Base: TBase) {
225256
})
226257
);
227258

228-
return !invalid;
259+
return false;
260+
}
261+
262+
const invalidId =
263+
replace.new.element.hasAttribute('id') &&
264+
replace.new.element.getAttribute('id') !==
265+
replace.old.element.getAttribute('id') &&
266+
Array.from(
267+
replace.new.element.ownerDocument.querySelectorAll(
268+
'LNodeType, DOType, DAType, EnumType'
269+
)
270+
).some(
271+
elm =>
272+
elm.getAttribute('id') ===
273+
(<Element>replace.new.element).getAttribute('id')
274+
);
275+
276+
if (invalidId) {
277+
this.dispatchEvent(
278+
newLogEvent({
279+
kind: 'error',
280+
title: get('editing.error.update', {
281+
name: replace.new.element.tagName,
282+
}),
283+
message: get('editing.error.idClash', {
284+
id: replace.new.element.getAttribute('id')!,
285+
}),
286+
})
287+
);
288+
289+
return false;
290+
}
291+
292+
return true;
229293
}
230294

231295
private onReplace(action: Replace) {
@@ -255,15 +319,15 @@ export function Editing<TBase extends LitElementConstructor>(Base: TBase) {
255319
private checkUpdateValidity(update: Update): boolean {
256320
if (update.checkValidity !== undefined) return update.checkValidity();
257321

258-
const invalid = Array.from(
322+
const invalidNaming = Array.from(
259323
update.element.parentElement?.children ?? []
260324
).some(
261325
elm =>
262326
elm.tagName === update.element.tagName &&
263327
elm.getAttribute('name') === update.newAttributes['name']
264328
);
265329

266-
if (invalid)
330+
if (invalidNaming) {
267331
this.dispatchEvent(
268332
newLogEvent({
269333
kind: 'error',
@@ -278,7 +342,34 @@ export function Editing<TBase extends LitElementConstructor>(Base: TBase) {
278342
})
279343
);
280344

281-
return !invalid;
345+
return false;
346+
}
347+
348+
const invalidId =
349+
update.newAttributes['id'] &&
350+
Array.from(
351+
update.element.ownerDocument.querySelectorAll(
352+
'LNodeType, DOType, DAType, EnumType'
353+
)
354+
).some(elm => elm.getAttribute('id') === update.newAttributes['id']);
355+
356+
if (invalidId) {
357+
this.dispatchEvent(
358+
newLogEvent({
359+
kind: 'error',
360+
title: get('editing.error.update', {
361+
name: update.element.tagName,
362+
}),
363+
message: get('editing.error.idClash', {
364+
id: update.newAttributes['id']!,
365+
}),
366+
})
367+
);
368+
369+
return false;
370+
}
371+
372+
return true;
282373
}
283374

284375
private onUpdate(action: Update) {

‎src/translations/de.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ export const de: Translations = {
116116
duplicate: 'Konnte {{name}} nicht kopieren',
117117
nameClash:
118118
'{{ parent }} enthält bereits ein {{ child }} Kind namens "{{ name }}"',
119+
idClash: 'Das Projekt enthält bereits die ID "{{ id }}"',
119120
},
120121
},
121122
validator: {

‎src/translations/en.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ export const en = {
113113
duplicate: 'Could not copy {{ name }}',
114114
nameClash:
115115
'Parent {{ parent }} already contains a {{ child }} named "{{ name }}"',
116+
idClash: 'The project has already an ID "{{ id }}"',
116117
},
117118
},
118119
textfield: {

‎test/testfiles/Editing.scd‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,12 @@
3838
</Bay>
3939
</VoltageLevel>
4040
</Substation>
41+
<DataTypeTemplates>
42+
<LNodeType id="testId" lnClass="LLN0">
43+
<DO name="Test" type="pointerToNowhere"/>
44+
</LNodeType>
45+
<DAType id="testId1">
46+
<BDA name="test" type="pointerToNowhere"/>
47+
</DAType>
48+
</DataTypeTemplates>
4149
</SCL>

‎test/unit/Editing.test.ts‎

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@ import { createUpdateAction, newActionEvent } from '../../src/foundation.js';
77

88
describe('EditingElement', () => {
99
let elm: MockEditor;
10+
let doc: XMLDocument;
1011
let parent: Element;
1112
let element: Element;
1213
let reference: Node | null;
1314

1415
beforeEach(async () => {
15-
const doc = await fetch('/test/testfiles/Editing.scd')
16+
doc = await fetch('/test/testfiles/Editing.scd')
1617
.then(response => response.text())
1718
.then(str => new DOMParser().parseFromString(str, 'application/xml'));
1819
elm = <MockEditor>(
@@ -114,6 +115,22 @@ describe('EditingElement', () => {
114115
expect(parent.querySelectorAll('Bay[name="Q01"]').length).to.be.equal(1);
115116
});
116117

118+
it('does not creates an element on id attribute conflict', () => {
119+
const newElement = elm.doc!.createElement('DOType');
120+
newElement?.setAttribute('id', 'testId');
121+
122+
elm.dispatchEvent(
123+
newActionEvent({
124+
new: {
125+
parent: doc.querySelector('DataTypeTemplates')!,
126+
element: newElement,
127+
reference: null,
128+
},
129+
})
130+
);
131+
expect(doc.querySelector('DOType')).to.be.null;
132+
});
133+
117134
it('deletes an element on receiving a Delete action', () => {
118135
elm.dispatchEvent(
119136
newActionEvent({
@@ -163,7 +180,7 @@ describe('EditingElement', () => {
163180
expect(testNode.parentNode).to.null;
164181
});
165182

166-
it('replaces an element on receiving an Delete action', () => {
183+
it('replaces an element on receiving an Replace action', () => {
167184
elm.dispatchEvent(
168185
newActionEvent({
169186
old: {
@@ -201,6 +218,46 @@ describe('EditingElement', () => {
201218
).to.equal(parent.querySelector('Bay[name="Q02"]'));
202219
});
203220

221+
it('replaces id defined element on receiving Replace action', () => {
222+
expect(doc.querySelector('LNodeType[id="testId"]')).to.not.be.null;
223+
224+
const newElement = doc.createElement('LNodeType');
225+
newElement?.setAttribute('id', 'testId3');
226+
227+
elm.dispatchEvent(
228+
newActionEvent({
229+
old: {
230+
element: doc.querySelector('LNodeType[id="testId"]')!,
231+
},
232+
new: {
233+
element: newElement,
234+
},
235+
})
236+
);
237+
expect(doc.querySelector('LNodeType[id="testId"]')).to.be.null;
238+
expect(doc.querySelector('LNodeType[id="testId3"]')).to.not.be.null;
239+
});
240+
241+
it('does not replace an element with name conflict', () => {
242+
expect(doc.querySelector('LNodeType[id="testId"]')).to.not.be.null;
243+
244+
const newElement = elm.doc!.createElement('LNodeType');
245+
newElement?.setAttribute('id', 'testId1');
246+
247+
elm.dispatchEvent(
248+
newActionEvent({
249+
old: {
250+
element: doc.querySelector('LNodeType[id="testId"]')!,
251+
},
252+
new: {
253+
element: newElement,
254+
},
255+
})
256+
);
257+
expect(doc.querySelector('LNodeType[id="testId"]')).to.not.be.null;
258+
expect(doc.querySelector('LNodeType[id="testId1"]')).to.be.null;
259+
});
260+
204261
it('moves an element on receiving a Move action', () => {
205262
elm.dispatchEvent(
206263
newActionEvent({
@@ -278,6 +335,20 @@ describe('EditingElement', () => {
278335
expect(element).to.not.have.attribute('desc');
279336
});
280337

338+
it('not update an element with id conflict', () => {
339+
const newAttributes: Record<string, string | null> = {};
340+
newAttributes['id'] = 'testId1';
341+
342+
elm.dispatchEvent(
343+
newActionEvent(
344+
createUpdateAction(doc.querySelector('LNodeType')!, newAttributes)
345+
)
346+
);
347+
348+
expect(doc.querySelector('LNodeType[id="testId"]')).to.exist;
349+
expect(doc.querySelector('LNodeType[id="testId1"]')).to.not.exist;
350+
});
351+
281352
it('does not update an element with name conflict', () => {
282353
const newAttributes: Record<string, string | null> = {};
283354
newAttributes['name'] = 'Q02';
@@ -291,6 +362,20 @@ describe('EditingElement', () => {
291362
expect(element).to.have.attribute('desc', 'Bay');
292363
});
293364

365+
it('does not update an element with id conflict', () => {
366+
const newAttributes: Record<string, string | null> = {};
367+
newAttributes['id'] = 'testId1';
368+
369+
elm.dispatchEvent(
370+
newActionEvent(
371+
createUpdateAction(doc.querySelector('LNodeType')!, newAttributes)
372+
)
373+
);
374+
375+
expect(doc.querySelector('LNodeType[id="testId"]')).to.exist;
376+
expect(doc.querySelector('LNodeType[id="testId1"]')).to.not.exist;
377+
});
378+
294379
it('carries out subactions sequentially on receiving a ComplexAction', () => {
295380
const child3 = elm.doc!.createElement('newBay');
296381
elm.dispatchEvent(

0 commit comments

Comments
 (0)