beta3 blocker: Usability and string freeze: rename admin/logs to admin/reports
dww - August 7, 2007 - 07:01
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | base system |
| Category: | task |
| Priority: | critical |
| Assigned: | dww |
| Status: | closed |
Description
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.
#1
+1 from me. I'm all for giving the menus clearer names and reports is much better, IMO.
Michelle
#2
+1. Reports is much better. Less geeky and just a better term for what you'll find there.
#3
This 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.
#4
use url() or l() for urls
index.php?q=admin/reports/dblogor basepath may not work.#5
and try to move URLs out of translatable strings and replace them with url placeholders...
#6
@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.
#7
this patch contains such a link, too.
- $log_message = ' All errors have been <a href="index.php?q=admin/logs/dblog">logged</a>.';+ $log_message = ' All errors have been <a href="index.php?q=admin/reports/dblog">logged</a>.';
#8
Here'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.
#9
176805 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 :)
#10
Thanks 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... ;)
#11
patch #8 patched
function system_update_177()#12
patch rerolled.
#13
@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.
#14
this improves the usability, I hope it gets in soon.
#15
Discussed this with Dries and we agree that this should be brought forward. It will be committed. Needs testing still.
#16
Great, 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...
#17
Re-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.
#18
I grepped through the Drupal source and found one minor admin/logs reference in system.install that needed to be converted.
#19
@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...
#20
dww: 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.
#21
Thanks, committed. The update docs need to have this change listed, so marking as code needs work.
#22
Documented at http://drupal.org/node/114774#admin_logs_to_admin_reports
#23
Excellent, thanks folks!
#24
Automatically closed -- issue fixed for two weeks with no activity.