Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
When moving update_status into D6 as update.module (http://drupal.org/node/94154), Dries pointed out that admin/logs/* didn't seem like an obvious place to look for the available updates page, but I pointed out it's right next to the status report. We agreed that renaming this section of the admin menu to be "admin/reports" makes much more sense. The log pages you get to from there all seem like reports already ("Top pages", "Recent log entries", "Recent hits", etc), and the "Status report" and "Available updates" fit more naturally.
Comment | File | Size | Author |
---|---|---|---|
#18 | logs_reports.patch | 28.05 KB | ChrisKennedy |
#17 | logs-reports-2.patch | 26.04 KB | JirkaRybka |
#12 | logs-reports.txt | 26.11 KB | rstamm |
#8 | 165140_reports.patch_2.txt | 29.08 KB | dww |
#3 | 165140_reports.patch.txt | 29.07 KB | dww |
Comments
Comment #1
Michelle+1 from me. I'm all for giving the menus clearer names and reports is much better, IMO.
Michelle
Comment #2
yoroy CreditAttribution: yoroy commented+1. Reports is much better. Less geeky and just a better term for what you'll find there.
Comment #3
dwwThis patch assumes you've already applied the fix for http://drupal.org/node/176805. If you don't apply that first, you'll get a single hunk that fails against update.php, but that's minor and can basically be ignored if you still want to review/test everything else.
Also, please note that this patch does *not* fix http://drupal.org/node/139015. I'll re-roll that patch when/if this one lands.
Comment #4
hass CreditAttribution: hass commenteduse url() or l() for urls
index.php?q=admin/reports/dblog
or basepath may not work.Comment #5
hass CreditAttribution: hass commentedand try to move URLs out of translatable strings and replace them with url placeholders...
Comment #6
dww@hass: your review is of the wrong patch, but i see you commented in http://drupal.org/node/176805, too, which is where this code originates. setting this back to needs review, since everything else in here is still valid.
Comment #7
hass CreditAttribution: hass commentedthis patch contains such a link, too.
Comment #8
dwwHere's a new patch based on the latest from http://drupal.org/node/176805
@hass: for the 3rd time...
*sigh*
Please either understand the issues and the patches, or leave them alone. Thanks.
Comment #9
Gábor Hojtsy176805 is resolved now, so this patch can stand on its own.
While I am fine with renaming admin/logs to admin/reports, it can be taken as a usability / simplification fix. I am not sure it is the best idea to do it now, but if carefully done, it could work well, IMHO. Still, if I were dww, I would wait for Dries to approve :)
Comment #10
dwwThanks for committing that other patch, Gabor, makes this one easier to work with. Yup, #8 still applies (with minor offsets). But yeah, let's see what Dries has to say about this... ;)
Comment #11
rstamm CreditAttribution: rstamm commentedpatch #8 patched
function system_update_177()
Comment #12
rstamm CreditAttribution: rstamm commentedpatch rerolled.
Comment #13
dww@Ralf, thanks good catch. However, we're still waiting for Dries to at least comment here about his intentions before I spend any more time getting reviews and tests for this. I'm tired of wasting time on core patches that aren't going to be committed.
Comment #14
rstamm CreditAttribution: rstamm commentedthis improves the usability, I hope it gets in soon.
Comment #15
Gábor HojtsyDiscussed this with Dries and we agree that this should be brought forward. It will be committed. Needs testing still.
Comment #16
dwwGreat, glad to see it's going to get committed. #12 still applies (with minor fuzz) and in my testing seems to work fine. Obviously, we could use some other testers and reviewers in here. If this is going to make it in for D6, we need to bump the visibility and priority of this, to hopefully get more eyes on the patch...
Comment #17
JirkaRybka CreditAttribution: JirkaRybka commentedRe-rolled to remove the fuzz, otherwise the same patch.
I read the patch file, did a fresh 6.x-dev install with the patch applied, enabled all modules affected by the patch, and browsed all logs/reports pages and other admin pages which I thought may be related, checking for links to logs/reports working. All fine, and no more places to be updated, as far as I can see (which is NOT all pages existing in the UI, no time to check all).
I think this may be RTBC, but preferrably with some more review.
Comment #18
ChrisKennedy CreditAttribution: ChrisKennedy commentedI grepped through the Drupal source and found one minor admin/logs reference in system.install that needed to be converted.
Comment #19
dww@ChrisKennedy: That's exactly what comment #12 was about. You (re)changed system_update_177(), but at the time system_update_177() runs, it is still going to be admin/logs, not admin/reports.
So, #18 should be ignored, #17 is our current patch to review...
Comment #20
ChrisKennedy CreditAttribution: ChrisKennedy commenteddww: ah, sorry about that, should have read the issue more thoroughly.
Anyway, I tested a D5 -> D6 upgrade and the relevant links for #17; everything worked fine.
Comment #21
Gábor HojtsyThanks, committed. The update docs need to have this change listed, so marking as code needs work.
Comment #22
ChrisKennedy CreditAttribution: ChrisKennedy commentedDocumented at http://drupal.org/node/114774#admin_logs_to_admin_reports
Comment #23
dwwExcellent, thanks folks!
Comment #24
(not verified) CreditAttribution: commentedAutomatically closed -- issue fixed for two weeks with no activity.