Standardize terminology on either 'status' or 'state', not both.
dww - January 1, 2009 - 20:31
| Project: | Project issue tracking |
| Version: | 6.x-1.x-dev |
| Component: | User interface |
| Category: | bug report |
| Priority: | normal |
| Assigned: | willmoy |
| Status: | needs review |
| Issue tags: | 6.x-1.0 blocker |
Description
While looking through the code working on something else, I noticed that in some places, we call "active", "fixed", etc the "Status" of an issue. I believe we're fairly consistent about that in the UI, otherwise, I'd call this a bug. However, the underlying code sometimes calls it a "status" and other times calls it a "state". We should standardize completely, rename functions + variables, etc. We should do this during/after the D6 port.
#1
Bah, this is a UI bug, since there are a few places we use "state" in the UI:
A) At node/N/edit/issues, we use "States" as the header above the checkboxes, and mention "states" in the help text.
B) The help text for the "Auto-close days" setting at admin/project/project-issue-settings
C) The URL for issue query pages uses "states=1,2,3..." as the GET parameter name
I still think we should only fix this in D6, since it's going to cause URL link rot in the case of (C), which we're going to face anyway as we convert all issue listings to views.
#2
Adding 6.x-1.0 blocker tag.
#3
There are—
324 uses of 'state' in project_issue
178 left after you replace the strings as in patterns.sed
32 left after you ignore $form_state
Those 32 are mainly natural language (comments, t() strings) and I can patch those manually.
The rest can be solved by running:
find project_issue_HEAD/ -type f -exec sed -i -f patterns.sed {} \;
patterns.sed is attached as patterns_sed.txt. A patch against HEAD is also attached, if this is a convenient time to apply it.
I don't have an install I can test the patch on but I have proof read it carefully.
This patch is by way of meagre thanks for all your hard work on this system.
#4
#5
Very cool, thanks! One glaring omission from this patch is that if the following happens:
- $result = db_query('SELECT * FROM {project_issue_state} ORDER BY weight');- while ($state = db_fetch_object($result)) {
- $options[] = $state;
+ $result = db_query('SELECT * FROM {project_issue_status} ORDER BY weight');
+ while ($status = db_fetch_object($result)) {
+ $options[] = $status;
then we definitely need to add a project_issue_update_6003() to rename this table. ;)
Otherwise, quick skim looks pretty good... Don't have time right now for a more in depth review, but this is off to a great start, especially since it's automated as a sed script, not a big patch to manually reroll all the time. ;)
Thanks again!
-Derek
#6
Right you are. You see, if you want something done properly, you have to do it yourself ;)
Patch attached, for the table rename on its own. New function:
<?php/**
* Rename {project_issue_state} to {project_issue_status}
*/
function project_issue_update_6003() {
$ret = array();
db_rename_table($ret, 'project_issue_state', 'project_issue_status');
return $ret;
}
?>
#7
Great, thanks. ;) Back to "needs review"...
#8
Anyone get a chance to test these patches yet?