Skip to content

Commit 7841f57

Browse files
lucaswerkmeisterlucaswerkmeister
committed
Use wbparsevalue when editing statement
Instead of directly using the text input value as the data value, send the user input to the API and use the returned data value. One visible benefit of this change is that user input with leading or trailing whitespace will now be silently trimmed by wbparsevalue, rather than result in an error when we send it untrimmed to wbeditentity. This is kind of the most bare-bones version of this change, only wiring the feature into the editStatementsStore. Future improvements for follow-up changes include, but are not limited to: - kick off the wbparsevalue requests as soon as the user types ("input" event, or perhaps "change" event?), instead of only on save - disable the submit button while the wbparsevalue request is pending (the "resolved" property is meant to make that available synchronously) - disable the submit button if the parsed value is the same as the current value (no change, e.g. only whitespace changes) - disable the submit button if parsing the value failed (or show an error message in that case? try again after a backoff?) In the editStatementsStore, I moved statementsForSerialization and buildStatementObjectFromMutableStatement into the actions so that they can access the right store even though there are now potentially `await`s between the different store calls. Bug: T405238 Change-Id: I66077e99305d5da3284713b838dadd74869164ea
1 parent d13160c commit 7841f57

File tree

6 files changed

+152
-37
lines changed

6 files changed

+152
-37
lines changed

‎cypress/e2e/wbui2025/publishStatementChanges.cy.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ describe( 'wbui2025 item view publish statement changes', () => {
8686
const valueForm = new ValueForm( element );
8787
valueForm.valueInput().invoke( 'val' ).should( 'equal', initialPropertyValue );
8888
editFormPage.publishButton().should( 'not.be.disabled' );
89-
valueForm.setValueInput( newPropertyValue );
89+
valueForm.setValueInput( ` ${ newPropertyValue } ` ); // should trim whitespace using wbparsevalue
9090
/* Change the rank */
9191
valueForm.setRank( 'Deprecated rank' );
9292
} );

‎repo/includes/RepoHooks.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -998,6 +998,7 @@ public function onResourceLoaderRegisterModules( $rl ): void {
998998
'resources/wikibase.wbui2025/api/editEntity.js',
999999
'resources/wikibase.wbui2025/store/editStatementsStore.js',
10001000
'resources/wikibase.wbui2025/store/messageStore.js',
1001+
'resources/wikibase.wbui2025/store/parsedValueStore.js',
10011002
'resources/wikibase.wbui2025/store/savedStatementsStore.js',
10021003
'resources/wikibase.wbui2025/store/serverRenderedHtml.js',
10031004
[

‎repo/resources/wikibase.wbui2025/api/editEntity.js‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,30 @@ const renderPropertyLinkHtml = async function ( propertyId ) {
3333
return fetchResult.wbformatentities && fetchResult.wbformatentities[ propertyId ];
3434
};
3535

36+
/**
37+
* Parse a given input into a full data value.
38+
*
39+
* @param {string} propertyId
40+
* @param {string} input
41+
* @returns {Promise<Object|null>} A data value object (with "type" and "value" keys),
42+
* or null if the input could not be parsed successfully.
43+
*/
44+
const parseValue = async function ( propertyId, input ) {
45+
try {
46+
const { results } = await api.get( {
47+
action: 'wbparsevalue',
48+
property: propertyId,
49+
values: [ input ]
50+
} );
51+
return results.find( ( result ) => result.raw === input );
52+
} catch ( e ) {
53+
return null;
54+
}
55+
};
56+
3657
module.exports = {
3758
updateStatements,
3859
renderSnakValueHtml,
39-
renderPropertyLinkHtml
60+
renderPropertyLinkHtml,
61+
parseValue
4062
};

‎repo/resources/wikibase.wbui2025/store/editStatementsStore.js‎

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
const { defineStore } = require( 'pinia' );
2+
const { useParsedValueStore } = require( './parsedValueStore.js' );
23
const { useSavedStatementsStore } = require( './savedStatementsStore.js' );
34
const { updateStatements } = require( '../api/editEntity.js' );
45

@@ -38,30 +39,6 @@ const useEditStatementStore = ( statementId ) => defineStore( 'editStatement-' +
3839
}
3940
} );
4041

41-
const buildStatementObjectFromMutableStatement = function ( statementId ) {
42-
const editStatementStore = useEditStatementStore( statementId )();
43-
const builtData = {
44-
id: statementId,
45-
mainsnak: {
46-
snaktype: editStatementStore.snaktype,
47-
property: editStatementStore.propertyId
48-
},
49-
references: editStatementStore.references,
50-
'qualifiers-order': editStatementStore.qualifiersOrder,
51-
qualifiers: editStatementStore.qualifiers,
52-
type: 'statement',
53-
rank: editStatementStore.rank
54-
};
55-
if ( editStatementStore.snaktype === 'value' ) {
56-
builtData.mainsnak.datavalue = {
57-
value: editStatementStore.value,
58-
type: 'string'
59-
};
60-
builtData.mainsnak.datatype = 'string';
61-
}
62-
return builtData;
63-
};
64-
6542
const useEditStatementsStore = defineStore( 'editStatements', {
6643
state: () => ( {
6744
statements: [],
@@ -104,25 +81,62 @@ const useEditStatementsStore = defineStore( 'editStatements', {
10481
this.removedStatements.push( removeStatementId );
10582
},
10683

84+
/**
85+
* @private
86+
* @param {string} statementId
87+
* @returns {Promise<object>}
88+
*/
89+
async buildStatementObjectFromMutableStatement( statementId ) {
90+
const parsedValueStore = useParsedValueStore();
91+
const editStatementStore = useEditStatementStore( statementId )();
92+
const builtData = {
93+
id: statementId,
94+
mainsnak: {
95+
snaktype: editStatementStore.snaktype,
96+
property: editStatementStore.propertyId
97+
},
98+
references: editStatementStore.references,
99+
'qualifiers-order': editStatementStore.qualifiersOrder,
100+
qualifiers: editStatementStore.qualifiers,
101+
type: 'statement',
102+
rank: editStatementStore.rank
103+
};
104+
if ( editStatementStore.snaktype === 'value' ) {
105+
builtData.mainsnak.datavalue = await parsedValueStore.getParsedValue(
106+
editStatementStore.propertyId,
107+
editStatementStore.value
108+
);
109+
builtData.mainsnak.datatype = 'string';
110+
}
111+
return builtData;
112+
},
113+
114+
/**
115+
* @private
116+
* @returns {Promise<object[]>}
117+
*/
118+
async buildStatementsForSerialization() {
119+
const claimsToDeleteOnSubmit = this.removedStatements
120+
.filter( ( statementId ) => !this.createdStatements.includes( statementId ) )
121+
.map( ( statementId ) => ( { id: statementId, remove: '' } ) );
122+
const statements = [];
123+
for ( const statementId of this.statements ) {
124+
statements.push( await this.buildStatementObjectFromMutableStatement( statementId ) );
125+
}
126+
return statements.concat( claimsToDeleteOnSubmit );
127+
},
128+
107129
/**
108130
* @param {string} entityId
109131
*/
110-
saveChangedStatements( entityId ) {
132+
async saveChangedStatements( entityId ) {
111133
const statementsStore = useSavedStatementsStore();
112-
return updateStatements( entityId, this.statementsForSerialization )
134+
return updateStatements( entityId, await this.buildStatementsForSerialization() )
113135
.then( ( returnedClaims ) => statementsStore.populateWithClaims( returnedClaims, true ) );
114136
}
115137
},
116138
getters: {
117-
statementIds: ( state ) => state.statements,
118-
statementsForSerialization( state ) {
119-
const claimsToDeleteOnSubmit = state.removedStatements
120-
.filter( ( statementId ) => !state.createdStatements.includes( statementId ) )
121-
.map( ( statementId ) => ( { id: statementId, remove: '' } ) );
122-
return state.statements
123-
.map( ( statementId ) => buildStatementObjectFromMutableStatement( statementId ) )
124-
.concat( claimsToDeleteOnSubmit );
125-
}
139+
statementIds: ( state ) => state.statements
126140
}
127141
} );
128142

‎repo/resources/wikibase.wbui2025/store/parsedValueStore.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
const { defineStore } = require( 'pinia' );
2+
const { parseValue } = require( '../api/editEntity.js' );
3+
4+
const useParsedValueStore = defineStore( 'parsedValue', {
5+
state: () => ( {
6+
parsedValuesPerProperty: new Map()
7+
} ),
8+
actions: {
9+
getParsedValue( propertyId, value ) {
10+
let parsedValues = this.parsedValuesPerProperty.get( propertyId );
11+
if ( parsedValues === undefined ) {
12+
parsedValues = new Map();
13+
this.parsedValuesPerProperty.set( propertyId, parsedValues );
14+
}
15+
let parsedValue = parsedValues.get( value );
16+
if ( parsedValue === undefined ) {
17+
parsedValue = {
18+
promise: parseValue( propertyId, value ).then( ( parsed ) => {
19+
parsedValue.resolved = parsed;
20+
return parsed;
21+
} ),
22+
resolved: undefined
23+
};
24+
parsedValues.set( value, parsedValue );
25+
}
26+
return parsedValue.promise;
27+
}
28+
}
29+
} );
30+
31+
module.exports = {
32+
useParsedValueStore
33+
};

‎repo/tests/jest/store/parsedValueStore.spec.js‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
jest.mock(
2+
'../../../resources/wikibase.wbui2025/api/editEntity.js',
3+
() => ( { parseValue: jest.fn() } )
4+
);
5+
6+
const { setActivePinia, createPinia } = require( 'pinia' );
7+
const { useParsedValueStore } = require( '../../../resources/wikibase.wbui2025/store/parsedValueStore.js' );
8+
const { parseValue: mockedParseValue } = require( '../../../resources/wikibase.wbui2025/api/editEntity.js' );
9+
10+
describe( 'parsed value store', () => {
11+
beforeEach( () => {
12+
setActivePinia( createPinia() );
13+
} );
14+
15+
it( 'sends one parseValue request with the right parameters', async () => {
16+
const parsedValueStore = useParsedValueStore();
17+
mockedParseValue.mockResolvedValueOnce( { type: 'string', value: 'abc' } );
18+
19+
const parsedValue1 = await parsedValueStore.getParsedValue( 'P123', ' abc ' );
20+
expect( parsedValue1 ).toEqual( { type: 'string', value: 'abc' } );
21+
expect( mockedParseValue ).toHaveBeenCalledWith( 'P123', ' abc ' );
22+
23+
const parsedValue2 = await parsedValueStore.getParsedValue( 'P123', ' abc ' );
24+
expect( parsedValue2 ).toEqual( { type: 'string', value: 'abc' } );
25+
expect( mockedParseValue ).toHaveBeenCalledTimes( 1 );
26+
} );
27+
28+
it( 'sends separate requests for different properties and values', async () => {
29+
const parsedValueStore = useParsedValueStore();
30+
mockedParseValue.mockImplementation( ( propertyId, value ) => Promise.resolve( {
31+
type: 'string',
32+
value: `parsed ${ propertyId }:${ value }`
33+
} ) );
34+
35+
const parsedValue1 = await parsedValueStore.getParsedValue( 'P123', 'abc' );
36+
const parsedValue2 = await parsedValueStore.getParsedValue( 'P456', 'def' );
37+
const parsedValue3 = await parsedValueStore.getParsedValue( 'P123', 'def' );
38+
39+
expect( mockedParseValue ).toHaveBeenCalledTimes( 3 );
40+
expect( parsedValue1 ).toEqual( { type: 'string', value: 'parsed P123:abc' } );
41+
expect( parsedValue2 ).toEqual( { type: 'string', value: 'parsed P456:def' } );
42+
expect( parsedValue3 ).toEqual( { type: 'string', value: 'parsed P123:def' } );
43+
} );
44+
45+
} );

0 commit comments

Comments
 (0)