- Issue created by @apmsooner
- @apmsooner opened merge request.
- Status changed to Needs review
about 2 years ago 5:26am 16 March 2023 The last submitted patch, 4: simple_decoupled_preview-n3348364-4.patch, failed testing. View results β
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.- Status changed to Needs work
about 2 years ago 6:43am 16 March 2023 - Status changed to Needs review
about 2 years ago 7:54am 16 March 2023 - Status changed to Needs work
about 2 years ago 2:16pm 16 March 2023 - πΊπΈUnited States DamienMcKenna NH, USA
Looks good. Two items:
1. The test failure seems legit - shouldn't the "page" item have the value "uid" because that's what's saved via the submitForm() method just a few lines higher up?
2. I think the assertNonAdminsCannotAccessForm() code could be moved inline, it doesn't serve much purpose being split out like that when other code isn't split out similarly. - Status changed to Needs review
about 2 years ago 2:36pm 16 March 2023 - πΊπΈUnited States apmsooner
- The build passes as is with "page" already having "uid"
$this->assertEquals(['article' => 'uid', 'page' => 'uid'], $config->get('includes'));
- Removed the function and moved code inline
- The build passes as is with "page" already having "uid"
- Status changed to RTBC
about 2 years ago 2:39pm 16 March 2023 -
apmsooner β
committed 7fa5cf68 on 1.0.x
Issue #3348364 by apmsooner, DamienMcKenna: Add test coverage for...
-
apmsooner β
committed 7fa5cf68 on 1.0.x
- Status changed to Fixed
about 2 years ago 2:44pm 16 March 2023 - Status changed to Fixed
about 2 years ago 6:08pm 17 March 2023