- Issue created by @ultimike
- First commit to issue fork.
- Merge request !11267Add a link to the status report from the database update results β (Open) created by cilefen
- πΊπΈUnited States nicxvan
The code changes look good, since this is a front end change a screenshot is needed I think.
I added the needs issue summary tag to get the standard issue template back, it's helpful to have all sections to review as issues progress.
I did confirm that the administration pages links are tested elsewhere.
- πΊπΈUnited States ultimike Florida, USA
@nicxvan, @cilefen,
Wow - thanks so much for hopping on this so quickly!
Some background on this request: each year at Florida DrupalCamp, during lightning talks, I share my list of "Drupal grievances" from the previous 12 months. The lack of the "Status Report" link was one of my original grievances a few years ago. At the time, I found a different core issue (which I have been unable to find again) that mentioned this, but its scope was bigger and never went anywhere.
Fast-forward to a few days ago, and I decided that this would not only make a good item for my "grievance rewind" for this year's lightning talk, but also a good issue to tackle (and possibly mentor someone new on) for Florida Drupal Camp's contribution day. Imagine my surprise (and joy!) when I opened this issue a few minutes ago to see the MR!
Hopefully, the narrow scope of this issue will allow this MR to be merged.
Marking as RTBC.
-mike
- πΊπΈUnited States nicxvan
I agree it's a good change, it's still needs work though because issues affecting the UI need before and after screenshots.
I added the default template back in.
If a section of the template is not applicable you can just leave it blank rather than deleting it, it makes it easier to see what is missing.
- πΊπΈUnited States ultimike Florida, USA
@nicxvan - just to clarify, are you saying that all that is currently needed is a before-and-after screenshot (and perhaps some better information in the issue template)?
-mike
- πΊπΈUnited States nicxvan
Yep! Just a before and after screenshot.
I already added the template back into the issue summary.
- πΊπΈUnited States nicxvan
Looks great! Not sure if this needs a change record or not.
- π¦πΊAustralia mstrelan
I have reviewed the MR and the code looks good to add the link to the page, however I suggest opening a follow up issue to convert both links to use Url::access instead of User::hasPermission. Currently if the required permission changed to "access site reports", for example, the test would still pass even though the link would not work. Since we're already using this pattern I suggest we keep it for now and get this one in. Looks good to me.
-
quietone β
committed 6d097f18 on 11.x
Issue #3508449 by cilefen, ultimike, nicxvan, mstrelan: Add link to...
-
quietone β
committed 6d097f18 on 11.x
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
quietone β credited larowlan β .
Automatically closed - issue fixed for 2 weeks with no activity.