Allow ordering of site map content

Thanks for contributing to Site map!

Please, if possible, provide your changes as a patch against the 7.x-1.x branch.

Documentation about patches http://drupal.org/patch

The best documentation about how to work with patches are at http://drupal.org/patch.

This is ABSOLUTELY useful.
I've tried it and I aprove it.

As suggested you should try to make a clean patch for that.

Thanks again ^^

Hey this is nice, anyone with some time want to roll the patch file? I want to put this in, as it is a cool option, but will not have time in the next few weeks.

Thanks

Did some major cleanup of the code, comments and UI. Should be easier to understand now. Also reduced some of the complexity of the ordering code. Patch attached against 1.x-dev.

It would be great if this had drag-and-drop ordering but I don't have the time to implement that right now.

@torpy and others, thanks for the patch, will give it a check.

Thanks again!

Torpy,

The patch applied fine, and the form items to order the items worked in the form (although I had 6 options and the count only went from 1-5), but in the output the numbers did not change the ordering. I tried several ways and cleared the cache for each test.

So I couldn't make it sort, although it did apply. Can you take another look at it?

Thank you,

Ouch, that's a pretty big issue, completely my fault: I was, for some unfathomable reason, using asort() instead of ksort(). Fixed that issue and the "n-1" options issue as well.

This patch should sort things out :)

Nice, thanks for the fast update.

Hey, I just was checking out drag and drop sorting on the weekend and had a test case working, so after we get yours in there, I'm interested in adding that for the interface.

Sounds great, if you need a hand just let me know!

Torpy,

Yep, applies and works and provides the functionality. It would be cool if this could be folded into the menu selection area (Menus to include in the site map) with drag and drop reordering. ( http://api.drupal.org/api/drupal/includes!common.inc/function/drupal_add... )

Then again, it works. This is a @frjo question since this module is his.

@frjo what do you think? Should we integrate this to sort the menus or do some fancy ordering to make the interface cleaner?

Thanks!

Good for me.
UI could be better but this is another point.

Oops ! I am sorry I forgot I had to reroll the patch ;)

Here it is. Turning back to "need review".

I was looking for something like this. Patch applies like it should and it works great, thanks for the addition!

How would I go about applying this patch?

http://drupal.org/patch/apply

I could only apply the patch in #16 by updating to latest dev branch and using patch -p1 < site_map-menu_ordering-15129708-16.patch. But it does work.

Yes, sounds like it applied. I'll take another look unless anyone else wants to grab it.

Thanks people!

Hello everyone,

Great module by the way! Could someone do me a favor by applying the latest patch to the 'site_map.tar_.gz' and send it to me as I am unable to apply any patches due to my host provider not allowing me access to SSH or run GIT, etc.

Cheers,
Jarbrain

The patch works fine for me. Thanks @BWPanda !

Patch applies well - it's a great addition I've had to do dirty hacks to get around before.

Patch is working for me. But I'm getting these notices on the site-map admin page:

Notice: Undefined index: front en site_map_admin_settings_form() (línea 135 de /Library/WebServer/Documents/todo/comer/sites/all/modules/site_map/site_map.admin.inc).
Notice: Undefined index: front en site_map_admin_settings_form() (línea 138 de /Library/WebServer/Documents/todo/comer/sites/all/modules/site_map/site_map.admin.inc).

Thanks for the patch!

I guess that the notices I mentioned on the site-map admin page are due to something on my side. I'll try to figure out and will report it if I find the problem.

Well I don't have Notices on site-map admin page anymore and I didn't do anything. I guess it was cache related. Sorry for my "spam".

Tested #22 works great Thank you.

+1 on #22, works great!

Apologies that I haven't had time to commit this as I'm off working on other projects

Is it possible to get a final review from a site_map maintainer to commit to dev?

Kudos! Looks like a nice improvement!

Patch is working fine (ordering the menus), but it needs an upgrade function. After applying the patch, the site map page is empty, until you go to its settings page (with tons a notices) and save the settings, then the page is filled again, and notices are gone.

Not maintaining this anymore, but looks like the patch needs just a little more work. Ordering is a nice feature, if the original patch author or anyone else of course needs the patch and has time to re-roll and fix, it would be great!

Thanks for the update @jdanthinne.

I've uploaded a new patch for the 7.x-1.2 release.

Tried to apply this but site map giving errors after the update.

@john.oltman: Could you be more specific? What is the error? a screenshot may help?

Similar problem as #36, I get erros after applying the patch #35. Attaching screenshot of the errors with this message.

Thanks!

I fixed above issues, here is the patch. This is a standalone patch you don't need to apply any other patches from this thread. It is working for site_map version 1.2
Unfortunately it's just quickfix for my project and I'm not sure if it is following all Drupal conventions related to patches (if there are any).

Erorros mentioned in #38 was caused by missing .js file and undefined initial values for sort rows. What is more I also fixed the issue with taxomony terms (they were always displayed in draggable table because of naming mistake).

Fixed one more issue discovered in previous patch connected with taxonomy terms loading by id instead of loading by name.

Anyone test this in the field yet? Thank you!

I'm also keen on getting this one tested and released, what can I do to help?

If you want to help, apply the patch #41 on your local install and report back if it worked. If it worked, then I can commit.

Tested: patch in #41 applied fine and tested a-ok. No errors in the log, and sorting worked. Like it, nice patch!

Should I integrate into dev or is someone else currently doing that. Thank you.

I'll commit to DEV within the next few hours.

Received these errors when using git apply -v for patch #41 against the tip of 7.x-1.x:

error: patch failed: site_map.module:436
error: site_map.module: patch does not apply

Was the patch generated using a git diff and the 7.x-1.x-dev branch (which is 4 commits ahead of 7.x-1.2)?

It seems that the patch was indeed against 7.x-1.2. The conflict was caused by:

commit 9506e4a6267c0fbd1fc4da90554753d3d30d38aa
Author: Amit Goyal <mintoo@gmail.com>
Date:   Tue May 13 18:41:05 2014 +0530

    minor coding standard issues fixed in site_map.module

On closer inspection I noticed that the hook drupal_alter call is gone ...

// Add an alter hook so that other modules can manipulate the
// menu tree prior to rendering.

$alter_mid = preg_replace('/[^a-z0-9_]+/', '_', $mid);
drupal_alter(array('site_map_menu_tree', 'site_map_menu_tree_' . $alter_mid), $tree, $menu);

Not sure if that was intended. Here's a re-roll of the patch against 7.x-1.x.

Committed and pushed #48. Please download the 7.x-1.x DEV branch after it builds and verify. I put the drupal_alter back in.

I downloaded the 7.x-1.x DEV branch twice. It seems the js-file is missing. After copying the js from the patch and uploading, ordering of the sitemap works.

@john.oltman: @artbussy seems to be right..

The issue is most likely because the patch in #48 does not apply cleanly. I first tried to apply it to 7.x-1.2, and I get an error message about a failed attempt to patch site_map.module for the _site_map_menus function (around line 443).

I then downloaded 7.x-1.x-dev per #50, and as mentioned by @artbussy and @helmo, the site_map.admin.js file is not included. It is necessary to create the JavaScript file using the code that's in patch #48. Once this file is in in the site_map module directory, the 7.x-1.x-dev version works.

Kicked out the previous commit. Let's wait until there's an associated JS file that works before putting this in.

Here's an updated patch that includes the JavaScript file. There's one addition in the newest 7.x-1.x-dev _site_map_menus function that wasn't there before:

// Allow other modules to alter it.
drupal_alter('site_map_menu_list', $mids);

I moved this to the template_preprocess_site_map function switch statement, since that's where the variable_get('site_map_show_menus', array()); now exists.

Not sure why the test bot is returning "Output: [error: site_map.admin.js: No such file or directory]."

I applied the patch locally without issue. Let me know if anyone has problems.

The patch seems to by trying to apply a *change* to site_map.admin.js - which is not present in 7.x-1.x
http://cgit.drupalcode.org/site_map/tree/?h=7.x-1.x

I *think* that you need to add -N to the args when creating the patch.

I would have expected the line where it goes

--- a/site_map.admin.js
+++ b/site_map.admin.js

to have been

--- /dev/null
+++ b/site_map.admin.js

or something.

But that's just an observation. I'm not suggesting editing it by hand.

That's odd, since I did "add -N" before creating the patch. Hmm... I'll try again and see if I can get it to build correctly.

Could not get add -N to show /dev/null, so I used format-patch instead. This worked.

@colan, I see that 7.x-1.3 was just released. This patch should be good to go... I fixed the associated javascript file a couple of months ago.

I see in the release notes that it was added and then removed... I'm assuming that was from the previous problems with the JS file. Is there anything that we need to do further with this? Thanks.

Someone else needs to successfully test it. When it gets to RTBC, I'll commit it.

I've applied the patch site_map-allow_ordering_content-1529708-61.patch (I had to manually set the base path, in order to get the patch working, but it worked fine and ordering looks good)
Let me know if you want me to submit any screenshots, or anything else.

@danieltome, the patch includes a new file (site_map.admin.js). This is normal behavior. Thanks for the review.

Patch in #62 applies cleanly and ordering is working.

Same for me. Thanks for the patch.

Thanks, anyone want to mark as RTBC?

Сommitted. Thanks!

\o/
Thanks :)

:-)

I don't believe a new release has been made in a while for this version with the ordering, can there be a 7.x-1.4 release with these changes or more like a parallel stable release with ordering?

Testing this, one potential issue may be when a site has too many resources to sort. Making an issue.

Installed the dev version of site map module today which has this ordering update included and works very well. Be great to see this in the main branch.