Skip to content

Commit 830876e

Browse files
iMarbotgodarkrai
iMarbot
and
godarkrai
authored
fix: priority always rejected + cleanup form validation (#11)
* fix: form rejected due to string/int type differences on priority * Update en.json * Use different submit button text for editing resource * fix typos * Remove unnecessary form validation * Simplify if statements * Show ID as locked field instead of hidden * Clean up i18n strings * Add help text to resource name field * Add min/max values to priority * Add filter callback function to trim inputs * Add validation callback function on resource input * remove unneeded manual validation * format code * format code * code style * Apply suggestions from code review Co-authored-by: Shashank Atreya <[email protected]> --------- Co-authored-by: Shashank Atreya <[email protected]>
1 parent 07dedec commit 830876e

File tree

2 files changed

+89
-142
lines changed

2 files changed

+89
-142
lines changed

‎i18n/en.json‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@
1010
"right-adminresourceloaderarticles":"Administrate ResourceLoader modules",
1111

1212
"resourceloaderarticles-cacheversion": "1",
13-
"resourceloaderarticles-show-list": "Show all pages",
14-
"resourceloaderarticles-add-page": "Add new page",
15-
"resourceloaderarticles-delete-page": "Delete page",
13+
"resourceloaderarticles-show-list": "Show all resources",
14+
"resourceloaderarticles-add-page": "Add new resource",
15+
"resourceloaderarticles-edit-page": "Save resource changes",
16+
"resourceloaderarticles-delete-page": "Delete resource",
1617
"resourceloaderarticles-edit": "Edit",
1718
"resourceloaderarticles-delete": "Delete",
18-
"resourceloaderarticles-page": "Page",
19+
"resourceloaderarticles-page": "Resource Page Name",
1920
"resourceloaderarticles-wiki": "Wiki",
2021
"resourceloaderarticles-type": "Type",
2122
"resourceloaderarticles-priority": "Priority",
@@ -24,14 +25,12 @@
2425
"resourceloaderarticles-resourcetype-script": "JavaScript",
2526
"resourceloaderarticles-resourcetype-style": "Style Sheet",
2627

27-
"resourceloaderarticles-error-page-empty": "The page parameter cannot be empty!",
28-
"resourceloaderarticles-error-page-invalid": "The page name is invalid!",
29-
"resourceloaderarticles-error-priority-empty": "The priority parameter cannot be empty!",
30-
"resourceloaderarticles-error-priority-invalid": "The priority is invalid - must be an integer!",
31-
"resourceloaderarticles-error-wiki-empty": "The wiki parameter cannot be empty!",
32-
"resourceloaderarticles-success-add": "Page successfully added",
33-
"resourceloaderarticles-success-edit": "Page successfully edited",
34-
"resourceloaderarticles-success-delete": "Page successfully deleted",
28+
"resourceloaderarticles-error-page-invalid": "The resource page name is invalid.",
3529

30+
"resourceloaderarticles-success-add": "Resource successfully added",
31+
"resourceloaderarticles-success-edit": "Resource successfully edited",
32+
"resourceloaderarticles-success-delete": "Resource successfully deleted",
33+
34+
"resourceloaderarticles-help-page": "Page name of resource (excl. `MediaWiki:Common.[js|css]/`). Valid JS ends with `.js` and valid styling ends with `.css` or `.less`.",
3635
"resourceloaderarticles-help-priority": "Priority for loading order of the resource (higher first), falls back to alphabetic order within a single priority class."
3736
}

‎src/SpecialPage/SpecialResourceLoaderArticles.php‎

Lines changed: 78 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,18 @@ private function addPage() {
113113
$formDescriptor = [
114114
'Page' => [
115115
'label-message' => 'resourceloaderarticles-page',
116+
'help-message' => 'resourceloaderarticles-help-page',
116117
'type' => 'text',
117118
'required' => true,
119+
'filter-callback' => [ $this, 'trimValueCB' ],
120+
'validation-callback' => [ $this, 'validatePageCB' ],
118121
],
119122
'Wiki' => [
120123
'label-message' => 'resourceloaderarticles-wiki',
121124
'type' => 'text',
122125
'required' => true,
123126
'default' => 'all',
127+
'filter-callback' => [ $this, 'trimValueCB' ],
124128
],
125129
'Type' => [
126130
'class' => 'HTMLSelectField',
@@ -136,6 +140,8 @@ private function addPage() {
136140
'type' => 'int',
137141
'required' => true,
138142
'default' => '0',
143+
'min' => -1000,
144+
'max' => 1000,
139145
],
140146
];
141147

@@ -151,69 +157,22 @@ private function addPage() {
151157
* @param array $formData
152158
*/
153159
public function addPageCB( $formData ) {
160+
$dbw = wfGetDB( DB_PRIMARY );
161+
$dbw->insert(
162+
'resourceloaderarticles',
163+
[
164+
'rla_page' => $formData[ 'Page' ],
165+
'rla_wiki' => $formData[ 'Wiki' ],
166+
'rla_type' => $formData[ 'Type' ],
167+
'rla_priority' => intval( $formData[ 'Priority' ] )
168+
]
169+
);
154170
$output = $this->getOutput();
155-
$store = true;
156-
if ( empty( $formData[ 'Page' ] ) ) {
157-
$output->addWikiTextAsContent(
158-
'<div class="error">'
159-
. $this->msg( 'resourceloaderarticles-error-page-empty' )->text()
160-
. '</div>'
161-
);
162-
$store = false;
163-
} elseif (
164-
(
165-
!( substr( $formData[ 'Page' ], -4 ) === '.css' || substr( $formData[ 'Page' ], -5 ) === '.less' )
166-
&& $formData[ 'Type' ] === 'style'
167-
)
168-
|| ( substr( $formData[ 'Page' ], -3 ) !== '.js' && $formData[ 'Type' ] === 'script' )
169-
) {
170-
$output->addWikiTextAsContent(
171-
'<div class="error">'
172-
. $this->msg( 'resourceloaderarticles-error-page-invalid' )->text()
173-
. '</div>'
174-
);
175-
$store = false;
176-
}
177-
if ( empty( $formData[ 'Wiki' ] ) ) {
178-
$output->addWikiTextAsContent(
179-
'<div class="error">'
180-
. $this->msg( 'resourceloaderarticles-error-wiki-empty' )->text()
181-
. '</div>'
182-
);
183-
$store = false;
184-
}
185-
if ( empty( $formData[ 'Priority' ] ) ) {
186-
$output->addWikiTextAsContent(
187-
'<div class="error">'
188-
. $this->msg( 'resourceloaderarticles-error-priority-empty' )->text()
189-
. '</div>'
190-
);
191-
$store = false;
192-
} elseif ( !is_int( $formData[ 'Priority' ] ) ) {
193-
$output->addWikiTextAsContent(
194-
'<div class="error">'
195-
. $this->msg( 'resourceloaderarticles-error-priority-invalid' )->text()
196-
. '</div>'
197-
);
198-
$store = false;
199-
}
200-
if ( $store ) {
201-
$dbw = wfGetDB( DB_PRIMARY );
202-
$dbw->insert(
203-
'resourceloaderarticles',
204-
[
205-
'rla_page' => $formData[ 'Page' ],
206-
'rla_wiki' => $formData[ 'Wiki' ],
207-
'rla_type' => $formData[ 'Type' ],
208-
'rla_priority' => $formData[ 'Priority' ]
209-
]
210-
);
211-
$output->addWikiTextAsContent(
212-
'<div class="success">'
213-
. $this->msg( 'resourceloaderarticles-success-add' )->text()
214-
. '</div>'
215-
);
216-
}
171+
$output->addWikiTextAsContent(
172+
'<div class="success">'
173+
. $this->msg( 'resourceloaderarticles-success-add' )->text()
174+
. '</div>'
175+
);
217176
}
218177

219178
/**
@@ -225,21 +184,27 @@ private function editPage( $id ) {
225184
$row = $res->fetchObject();
226185
$formDescriptor = [
227186
'Id' => [
228-
'type' => 'hidden',
187+
'label-message' => 'resourceloaderarticles-id',
188+
'type' => 'int',
229189
'required' => true,
190+
'disabled' => true,
230191
'default' => $row->rla_id,
231192
],
232193
'Page' => [
233194
'label-message' => 'resourceloaderarticles-page',
195+
'help-message' => 'resourceloaderarticles-help-page',
234196
'type' => 'text',
235197
'required' => true,
236198
'default' => $row->rla_page,
199+
'filter-callback' => [ $this, 'trimValueCB' ],
200+
'validation-callback' => [ $this, 'validatePageCB' ],
237201
],
238202
'Wiki' => [
239203
'label-message' => 'resourceloaderarticles-wiki',
240204
'type' => 'text',
241205
'required' => true,
242206
'default' => $row->rla_wiki,
207+
'filter-callback' => [ $this, 'trimValueCB' ],
243208
],
244209
'Type' => [
245210
'class' => 'HTMLSelectField',
@@ -256,11 +221,13 @@ private function editPage( $id ) {
256221
'type' => 'int',
257222
'required' => true,
258223
'default' => $row->rla_priority,
224+
'min' => -1000,
225+
'max' => 1000,
259226
],
260227
];
261228

262229
$htmlForm = HTMLForm::factory( 'ooui', $formDescriptor, $this->getContext() );
263-
$htmlForm->setSubmitText( $this->msg( 'resourceloaderarticles-add-page' )->text() );
230+
$htmlForm->setSubmitText( $this->msg( 'resourceloaderarticles-edit-page' )->text() );
264231
$htmlForm->setFormIdentifier( 'editPageCB' );
265232
$htmlForm->setSubmitCallback( [ $this, 'editPageCB' ] );
266233

@@ -271,72 +238,25 @@ private function editPage( $id ) {
271238
* @param array $formData
272239
*/
273240
public function editPageCB( $formData ) {
241+
$dbw = wfGetDB( DB_PRIMARY );
242+
$dbw->update(
243+
'resourceloaderarticles',
244+
[
245+
'rla_page' => $formData[ 'Page' ],
246+
'rla_wiki' => $formData[ 'Wiki' ],
247+
'rla_type' => $formData[ 'Type' ],
248+
'rla_priority' => intval( $formData[ 'Priority' ] )
249+
],
250+
[
251+
'rla_id' => $formData[ 'Id' ]
252+
]
253+
);
274254
$output = $this->getOutput();
275-
$store = true;
276-
if ( empty( $formData[ 'Page' ] ) ) {
277-
$output->addWikiTextAsContent(
278-
'<div class="error">'
279-
. $this->msg( 'resourceloaderarticles-error-page-empty' )->text()
280-
. '</div>'
281-
);
282-
$store = false;
283-
} elseif (
284-
(
285-
!( substr( $formData[ 'Page' ], -4 ) === '.css' || substr( $formData[ 'Page' ], -5 ) === '.less' )
286-
&& $formData[ 'Type' ] === 'style'
287-
)
288-
|| ( substr( $formData[ 'Page' ], -3 ) !== '.js' && $formData[ 'Type' ] === 'script' )
289-
) {
290-
$output->addWikiTextAsContent(
291-
'<div class="error">'
292-
. $this->msg( 'resourceloaderarticles-error-page-invalid' )->text()
293-
. '</div>'
294-
);
295-
$store = false;
296-
}
297-
if ( empty( $formData[ 'Wiki' ] ) ) {
298-
$output->addWikiTextAsContent(
299-
'<div class="error">'
300-
. $this->msg( 'resourceloaderarticles-error-wiki-empty' )->text()
301-
. '</div>'
302-
);
303-
$store = false;
304-
}
305-
if ( empty( $formData[ 'Priority' ] ) ) {
306-
$output->addWikiTextAsContent(
307-
'<div class="error">'
308-
. $this->msg( 'resourceloaderarticles-error-priority-empty' )->text()
309-
. '</div>'
310-
);
311-
$store = false;
312-
} elseif ( !is_int( $formData[ 'Priority' ] ) ) {
313-
$output->addWikiTextAsContent(
314-
'<div class="error">'
315-
. $this->msg( 'resourceloaderarticles-error-priority-invalid' )->text()
316-
. '</div>'
317-
);
318-
$store = false;
319-
}
320-
if ( $store ) {
321-
$dbw = wfGetDB( DB_PRIMARY );
322-
$dbw->update(
323-
'resourceloaderarticles',
324-
[
325-
'rla_page' => $formData[ 'Page' ],
326-
'rla_wiki' => $formData[ 'Wiki' ],
327-
'rla_type' => $formData[ 'Type' ],
328-
'rla_priority' => $formData[ 'Priority' ]
329-
],
330-
[
331-
'rla_id' => $formData[ 'Id' ]
332-
]
333-
);
334-
$output->addWikiTextAsContent(
335-
'<div class="success">'
336-
. $this->msg( 'resourceloaderarticles-success-edit' )->text()
337-
. '</div>'
338-
);
339-
}
255+
$output->addWikiTextAsContent(
256+
'<div class="success">'
257+
. $this->msg( 'resourceloaderarticles-success-edit' )->text()
258+
. '</div>'
259+
);
340260
}
341261

342262
/**
@@ -348,13 +268,15 @@ private function deletePage( $id ) {
348268
$row = $res->fetchObject();
349269
$formDescriptor = [
350270
'Id' => [
351-
'type' => 'hidden',
271+
'label-message' => 'resourceloaderarticles-id',
272+
'type' => 'int',
352273
'required' => true,
353274
'disabled' => true,
354275
'default' => $row->rla_id,
355276
],
356277
'Page' => [
357278
'label-message' => 'resourceloaderarticles-page',
279+
'help-message' => 'resourceloaderarticles-help-page',
358280
'type' => 'text',
359281
'required' => true,
360282
'disabled' => true,
@@ -409,4 +331,30 @@ public function deletePageCB( $formData ) {
409331
);
410332
}
411333

334+
/**
335+
* @param string $value
336+
* @return string
337+
*/
338+
public function trimValueCB( $value ) {
339+
return trim( $value );
340+
}
341+
342+
/**
343+
* @param string $value
344+
* @param array $alldata
345+
* @return bool|string
346+
*/
347+
public function validatePageCB( $value, $alldata ) {
348+
if (
349+
( $alldata[ 'Type' ] === 'style'
350+
&& !( ( strlen( $value ) > 4 && substr( $value, -4 ) === '.css' )
351+
|| ( strlen( $value ) > 5 && substr( $value, -5 ) === '.less' )
352+
)
353+
) || ( $alldata[ 'Type' ] === 'script' && !( strlen( $value ) > 3 && substr( $value, -3 ) === '.js' ) )
354+
) {
355+
return $this->msg( 'resourceloaderarticles-error-page-invalid' )->text();
356+
}
357+
return true;
358+
}
359+
412360
}

0 commit comments

Comments
 (0)