Submitting a patch that adds two straightforward options. One disables the automatic creation of URL aliases (i.e. in the case that you want to use pathauto), and the other disables the automatic project overview page at the project/ URL (i.e. in the case that you want to use a view).

Comments

dww’s picture

Status: Active » Needs review

looks pretty good to me. but, i'm not using my normal workstation, so i can't actually test this well right now. will take another look when i have access to my test sites again.

dww’s picture

Status: Needs review » Needs work

still haven't tested this, but i noticed a minor gripe with the code style: True should be TRUE. otherwise, this really should be RTBC.

aclight’s picture

--------------------------
File to patch: project.module
patching file project.module
Hunk #1 succeeded at 246 with fuzz 2 (offset 129 lines).
Hunk #2 succeeded at 337 with fuzz 2 (offset 110 lines).
Hunk #3 FAILED at 467.
1 out of 3 hunks FAILED -- saving rejects to file project.module.rej

Note also that this patch creates a new variable, but does not include code in project.install to delete the variable in hook_uninstall(). That function might not have existed when this patch was originally written, but it's there now.

agentrickard’s picture

Status: Needs work » Needs review
StatusFileSize
new2.96 KB

The 'project' path is a normal menu item and can be disabled via the menu system.

Turned off path aliasing makes sense to me, patch attached.

aclight’s picture

I could be wrong, and haven't checked the code, but it seems like we'd need some additional code in project_project_breadcrumb (or whatever that function is called), since I seem to recall that this function relies on the path of projects being /project/

agentrickard’s picture

StatusFileSize
new41.2 KB
new41.47 KB

Does not appear to be an issue. The attached screenshots show the output of project_project_set_breadcrumb() with the patch applied and not. The menu system recognizes the project node as part of the Project* menu hierarchy in both cases.

aclight’s picture

Title: Flags to disable auto alias and overview creation » Add setting to disable auto alias and overview creation
StatusFileSize
new3.35 KB

This looks good to me, but I have a few comments:

1. Do we want to wrap:

+  $form['project_enable_alias'] = array(
+    '#type' => 'checkbox', '#title' => t('Enable auto-generated project/[name] alias'),
+    '#default_value' => variable_get('project_enable_alias', TRUE),
+    '#description' => t('If checked, project module will automatically generate path aliases for each project. Uncheck if you want to use pathauto, for example.')
+  );

in something like:

if (module_exists('path')) {
}

Because if the path module isn't enabled, the setting will make no sense because aliases won't be generated anyway.

2. I fixed a code style error in the patch and reworded the title and description of the setting to be (IMO) more clear.

3. The description text for the Project short name (URI) element on the project node form is this:
"This will be used to generate a /project// URL for your project. This string is also used to generate the name of releases associated with this project."

However, if this new setting is disabled, an alias as such won't actually be created. This would also be the case in the module as it is now if the path module is disabled. Do we want to somehow change this description to be more clear? I'm not sure it's necessary, but for example if the user wanted to use pathauto to create aliases at /project/foo//, then I think the description in that field could confuse project owners.

aclight’s picture

Status: Needs review » Needs work

Also, what about the code in project_project_view(). $project->uri is used directly there to create links, and that's going to cause a problem if the shortname isn't aliased as expected, I believe. Setting to CNW for that reason.

agentrickard’s picture

Status: Needs work » Needs review
StatusFileSize
new4.62 KB

Attached revision addresses the issues above, as follows:

1) Wrapped in a check, and the variable is set to FALSE if path does not exist.
2) Incorporated.
3) Fixed by adding variant text based on the setting. Also required a modification to project_release_alter_project_form().

As for the links in project_project_view(), these are not affected. $node->uri is actually the stored value of the short name from the form. These do not seem to be path aliases. Instead they are links that are interpreted by project_project_retrieve(), which accepts either the short name or the node id of the project.

dww’s picture

Version: 5.x-0.1-beta » 6.x-1.x-dev
Status: Needs review » Needs work

I'd love to get this in before 6.x-1.0 (and I'd probably be willing to commit a backport to D5 if anyone still cares). Anyone up for a re-roll?

fuzzy_texan’s picture

dww’s picture

Assigned: Unassigned » dww
Status: Needs work » Needs review
StatusFileSize
new5.9 KB

Rerolled for D6 in HEAD.

dww’s picture

Title: Add setting to disable auto alias and overview creation » Add setting to disable auto URL aliases for project nodes
Status: Needs review » Fixed

Committed to HEAD. I don't care enough for a backport to D5, but if anyone's interested, feel free to reopen this with a patch.

Status: Fixed » Closed (fixed)
Issue tags: -6.x-1.0 wishlist

Automatically closed -- issue fixed for 2 weeks with no activity.