This was an annoying glitch in the aggregator API.

Other functions go aggregator_[verb]_[thing being verbed], but for whatever reason load doesn't. Let's fix that.

Files: 
CommentFileSizeAuthor
#9 aggregator-load-order-4.patch1.95 KBkathyh
PASSED: [[SimpleTest]]: [MySQL] 33,068 pass(es).
[ View ]
#7 aggregator-load-order-3.patch2.47 KBkathyh
PASSED: [[SimpleTest]]: [MySQL] 33,001 pass(es).
[ View ]
#6 aggregator-load-order-3.patch2.47 KBkathyh
PASSED: [[SimpleTest]]: [MySQL] 32,997 pass(es).
[ View ]
#5 aggregator-load-order-2.patch2.47 KBkathyh
PASSED: [[SimpleTest]]: [MySQL] 33,001 pass(es).
[ View ]
aggregator-load-order.patch2.85 KBwebchick
FAILED: [[SimpleTest]]: [MySQL] 32,767 pass(es), 140 fail(s), and 0 exception(es).
[ View ]

Comments

Status:Needs review» Needs work

The last submitted patch, aggregator-load-order.patch, failed testing.

Issue tags:+Novice

Meh. I don't actually care enough to fix whatever's causing test failures myself, but I bet it's something stupid.

Tagging "novice".

Assigned:Unassigned» kathyh

Assigned to kathyh. Will take a look.

Status:Reviewed & tested by the community» Needs work

Novice notes:
GET ISSUE
-Select Issue from queue that is tagged with "Novice":
http://drupal.org/project/issues/search/drupal?issue_tags=Novice
-Assigned it to myself

IF IT COMES WITH A PATCH - APPLY PATCH
-downloaded new *AMP stack to get the php version for 8.x and then installed d8 site
-downloaded greasemonkey to better REVIEW the patch in the issue http://drupal.org/project/dreditor
-get a copy of drupal8 from git:
git clone http://git.drupal.org/project/drupal.git drupal8 (where drupal8 is the dir for the repo)
git checkout -b local 8.x
-to use the patch (from http://drupal.org/patch) downloaded cygwin
-install patch module and utils/cygutils (to get the unix2dos utility)
with the patch module to run: patch -p1 < patchfile

FIX THE ISSUE
-now that I have the patch, looking at test dir in the aggregator dir to see which test fail and why (e.g. fixing the issue)
-ask questions in #drupal on IRC http://webchat.freenode.net/
-ask questions in #drupal-contribute - see http://groups.drupal.org/node/175539

CREATE THE NEW PATCH
-after done will look at Making a Drupal Patch with GIT http://drupal.org/node/707484 - run unix2dos on files
git config --global core.autocrlf true
git config --global core.safecrlf true
git diff > aggregator-load-order-3.patch

UPLOAD PATCH AND TEST
-switch the status to "needs review" and upload the patch. This will wake up the test-bot... eventually.
-if it passes, then leave status as "need review" and wait for RTBC

StatusFileSize
new2.47 KB
PASSED: [[SimpleTest]]: [MySQL] 33,001 pass(es).
[ View ]

-function aggregator_load_feed removed from patch (left as *_load), uploading for test

StatusFileSize
new2.47 KB
PASSED: [[SimpleTest]]: [MySQL] 32,997 pass(es).
[ View ]

-re-upload (status was ignored)

Status:Needs work» Needs review
StatusFileSize
new2.47 KB
PASSED: [[SimpleTest]]: [MySQL] 33,001 pass(es).
[ View ]

ok - google says I need to change status to needs review - let's see if this calls the test bot

Status:Needs review» Needs work

@kathyh: Thanks for providing the patch!

There are 3 functions that match /aggregator_*_load/: aggregator_feed_load(), aggregator_category_load() and aggregator_feed_items_load().

The problem with renaming the first two functions is that they both are menu wildcard loaders, and sadly there is not a way to set manually the callback name for them, so they should not be removed.
Another possibility is to declare the right-named functions, and there call the menu wildcard loader functions manually, but that seems even more confuse to me.

--- a/modules/aggregator/aggregator.module
+++ b/modules/aggregator/aggregator.module
@@ -684,7 +684,7 @@ function aggregator_feed_load($fid) {
  * @return
  *   An associative array describing the category.
  */
-function aggregator_category_load($cid) {
+function aggregator_load_category($cid) {
   $categories = &drupal_static(__FUNCTION__);
   if (!isset($categories[$cid])) {
     $categories[$cid] = db_query('SELECT * FROM {aggregator_category} WHERE cid = :cid', array(':cid' => $cid))->fetchAssoc();

@kathyh: Based on the reasoning above, this hunk need to be removed, and after it, I guess this issue will be RTBC ;-)

Status:Needs work» Needs review
StatusFileSize
new1.95 KB
PASSED: [[SimpleTest]]: [MySQL] 33,068 pass(es).
[ View ]

@marvil07 - thanks for the feedback and help in #drupal - much appreciated.
Change was to remove menu wildcard loaders from the patch. Resubmitting.

Status:Needs review» Reviewed & tested by the community

As mentioned on #8, IMHO this is ready now :-)

Status:Needs work» Fixed

Committed to 8.x. Thanks.

Version:8.x-dev» 7.x-dev
Status:Fixed» Reviewed & tested by the community

The log indicates that this was committed to D8 and D7. The D7 commit must have been accidental.

Priority:Minor» Critical

Version:7.x-dev» 8.x-dev
Status:Reviewed & tested by the community» Fixed

Oh dear. :)

Rolled back in 7.x. Thanks for escalating, Eric_A, and thanks for the patch kathyh!!

Issue tags:+Needs change record

This needs a change notification, no?

@Tor Arne Thune:

This needs a change notification, no?

Yup. Follow your own link and create one; it's not hard. ;-)

Well, seeing as the issue is marked Novice, I was hoping to give a hint to someone stumbling upon the issue that has never done it before.

Title:s/aggregator_*_load/aggregator_load_*/gAPI change notice for s/aggregator_*_load/aggregator_load_*/g
Status:Fixed» Active

Status:Active» Needs review

For your review (this is my first change notice submitted... so let me know if I mucked something up).
http://drupal.org/node/1295398

It look fine :-), maybe(because target is developers for this change notice) mentioning that some functions matching the pattern mentioned on this issue title end up not changing because they are menu wildcard loaders(see comment 8 for more information) is relevant. Otherwise I guess it's ok, but I'm not an expert on change notices ;-)

Title:API change notice for s/aggregator_*_load/aggregator_load_*/gRename aggregator_feed_items_load() to aggregator_load_feed_items().

Rewrote the change notice. Only one function was renamed, although three were considered.

Also posted comments to 7.x and 8.x API pages.

Title:Rename aggregator_feed_items_load() to aggregator_load_feed_items().API change notice: Rename aggregator_feed_items_load() to aggregator_load_feed_items().

Readding change notice part of issue title so people realise that is all that is outstanding

Title:API change notice: Rename aggregator_feed_items_load() to aggregator_load_feed_items().Rename aggregator_feed_items_load() to aggregator_load_feed_items().
Status:Needs review» Fixed

Huh? No; the change notice has been written.

Status:Fixed» Closed (fixed)

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

Priority:Critical» Normal
Issue tags:-Novice, -Needs change record