As I posted my original feature request 2362333 in the wrong group at first, I guess that nobody over here noticed it when I moved it over. So – let's try to give this a little poke so perhaps someone will see it.

The long version can still be found over there. (UPDATE: I don't seem to be able to attach that issue as a related one. *headscratch*) The patch file also works on the latest 7.x-2.0-alpha8, I've re-attached it to this ticket.

What it does is: It sorts the list of feed importers by their readable names – which, compared to the machine name, can be changed to provoke a different feed list sort order. I am working on a huge database migration with about 30 feeds that need to be performed one by on in the correct order. This helps a lot.

It's really just one line of code in feeds.module, and it greatly increases clarity.

UPDATE:

Just for clarification, as twistor mentioned it: This is only about the sort order on the feeds overview page ./admin/structure/feeds and the manual import page ./import. It does not define a weight or a processing order – it's really just about having the feeds sorted alphanumerically by their visual names instead of by their internal machine name (which can't be changed later).

Comments

GerZah’s picture

Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, feeds.module_sorted-list.patch, failed testing.

megachriz’s picture

Title: Sorting feed importers by readable names (revisited) » Sorting feed importers by readable names
GerZah’s picture

Thanks, MegaChriz.

I'm apparently too stupid to create a patch file – at least one that suffices the requirements for automatic patching. I'm sorry, I'm not so versed with git, I simply modified my original version of feeds.module and created a diff between the original version and mine. Rest assured, the patch applies nicely to the 7.x-2.0-alpha8 version of feeds.module.

megachriz’s picture

Version: 7.x-2.0-alpha8 » 7.x-2.x-dev

@GerZah
I don't think you are stupid. Patches just sometimes fail tests. Even patches from the Drupal core maintainers sometimes fail tests.

I 'm changing the version to 7.x-2.x-dev, because patches for feature requests should always be tested against dev. I will trigger a re-test after that as at first glance the failing tests ("Undefined index: content-type") seem unrelated.

Status: Needs work » Needs review
GerZah’s picture

No – I said that I was too stupid to submit a working patch. ;-) I see that the patch now went through just fine. Thank you very much!

twistor’s picture

Status: Needs review » Needs work

We can't use anonymous functions since we depend on PHP >= 5.2.

Also, the strval() call is unnecessary.

Sorting the list by name is fine, I'm not sure how multi-byte characters will work.

That said, I'm not sure if this is the best idea. Feeds doesn't offer any guarantees about the order that importers execute. If you update an importer, it's possible that it will reschedule the feeds, and that would put things out of sync.

I've worked on this before, but I don't think I ever released any code. The idea is to mark importers as dependent on another importer, and have the parent importer be the one that is scheduled. That would ensure that they run in the correct order, but then that would only work for the standalone importer.

megachriz’s picture

@GerZah
If you know your way in object orientated code, you could also take a look at the Migrate module, which is designed especially for migrations and has good dependency handling (in the sense of one migration depends on an other). It takes some effort to grasp though and it usually takes longer to setup as you need to define the migrations and mapping in code (there are also some UI modules available to simplify the process, but I haven't tried these yet).

GerZah’s picture

@twistor:

  • Good point – I didn't see the PHP >= 5.2 issue when defining an anonymous function as a callback. That can be solved quite easily.
  • For some reason, I needed to typecast $a->config["name"] to string via strval() – it somehow didn't work without it; I agree, it shouldn't be necessary. When I re-implement this without the anonymous function call, I'll look into it.
  • Agreed – this approach definitely does not define any dependency between importers. It is not meant to define a processing order for scheduled importers, and it certainly doesn't.
  • However, what is does is: The uasort() is part of feeds_importer_load_all() – so this merely defines the visual sort order on ./admin/structure/feeds and ./import – that is, (a) when configuring the feeds and (b) when calling the non-periodic file-fetchers.
  • At the moment, the sort order in these two lists is defined by the feeds' machine names, which can not be changed later. This one additional line of code merely switches this to be sorted by the readable name instead. This does not define any "weight" and thus a processing order or anything like that.
  • In my case, the list tended to become extremely incomprehensible due to its seemingly random order. … Granted: As long as you are dealing with 3-5 feeds, this doesn't pose a problem. But if you are looking at 30-35 feeds (which will be run pretty much exactly once during the data migration), it is very helpful to be able to define a visual sort order.

@MegaChriz: Thanks for the suggestion. I'm far too far down the road to switch from feeds to migrate at this point. :)

GerZah’s picture

Issue summary: View changes
GerZah’s picture

OK – there we go:

  1. I don't use an anonymous function anymore but instead defined a local callback function.
  2. I got rid of the strval() calls – still works for me.

Better? ... This works for me with 7.x-2.x-dev / 2014-Dec-23.

GerZah’s picture

Status: Needs work » Needs review
GerZah’s picture

Errm.

Sorry, don't mind #12. Breaks due to (possible) re-definition. Bummer!

Try this one here in #14, please.

GerZah’s picture

jerenus’s picture

Status: Needs review » Reviewed & tested by the community

Pass the testing. Reroll the patch and add some comments. Look forward to this function to be committed.

jerenus’s picture

megachriz’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new832 bytes
new1.33 KB

Thanks for the patch, GerZah and Jerenus.

I've expanded the function documentation a bit and also moved the uasort() call to the place were all feeds importers are loaded. This ensures that we can expect FeedsImporter objects in feeds_importer_name_sort().

  • MegaChriz committed db3e28a on 7.x-2.x authored by GerZah
    Issue #2385601 by GerZah, MegaChriz, Jerenus: Sorting feed importers by...
megachriz’s picture

Status: Needs review » Fixed

Committed #18. Thanks all!

Status: Fixed » Closed (fixed)

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