WordPress.org

Ready to get started?Download WordPress

Forums

WP-Filebase Download Manager
Bug in file list with multiple categories (9 posts)

  1. ccalvert
    Member
    Posted 2 years ago #

    WordPress v. 3.2.1
    PHP v. 5.2.17
    WP-Filebase v. 0.2.9.2

    When using WP-Filebase to display a "file list" with multiple categories, only the files from the first category and the first category name are displayed.

    Shortcode: [wpfilebase tag=list id='4,1']

    Observed behavior: The category name and files for category 4 are displayed. The category name and files for category 1 are not, despite the fact that there are files in the category.

    Expected behavior: The category name and files for all specified categories should be displayed.

    Source of bug:

    In classes/ListTpl.php, WPFB_ListTpl::Generate() on line 120, the following line appears:

    if($n > $num) break; // TODO!!

    $n is an index declared before the loop is entered (line 113) and incremented inside the inner loop that iterates over the files (line 126). $num is an argument originating in WPFB_Core::ShortCode() on line 237 and is declared with a value of zero. It does not appear to be modified along the call chain to Generate(), and in any case always had a value of zero in my tests.

    Because $num is always zero, $n will exceed its value after one iteration of the inner foreach loop (line 124) and the condition on line 120 will be true on the second iteration of the outer foreach loop (line 114), causing categories 2..N to be skipped.

    The simplest patch is to comment out line 120.

    http://wordpress.org/extend/plugins/wp-filebase/

  2. GnuTux
    Member
    Posted 2 years ago #

    I can confirm this bug as well.

    I also noticed that the number of files are off by 1 when I choose to "List Selected Categories". For example, it says 6 files in the category when there are only 5.

  3. ccalvert
    Member
    Posted 2 years ago #

    Yes, I can confirm that the file count is off as well. I'm looking into the code to see why.

  4. ccalvert
    Member
    Posted 2 years ago #

    It looks like the problem is that Category::NotifyFileRemoved is not correctly decrementing cat_num_files and cat_num_files_total or is not being called when the category of a file is changed.

  5. ccalvert
    Member
    Posted 2 years ago #

    The problem appears to be Item::GetParent. Contrary to its name, it's not a getter, but a mutating accessor; in addition to returning the current parent class, it updates the last_parent to be the current parent.

    Aside from the obvious design problem (getters should not mutate), it's throwing away the historical information that one assumes the last_parent should provide. This functionality belongs in a setter, not a getter, and it should not update the last_parent to match the current parent, but rather the prior parent.

    What's happening is that when a file's category is changed in the edit form, the program updates the file's category, then notifies the category instance that a file has been removed. The category instance then calls Item::GetParent via Item::IsAncestorOf in order to make sure that the file in question was in fact previously a member of that category. However, by that point GetParent has already been called somewhere else and updated last_parent to be an instance of the new category instead of the old.

    This is a rather involved problem, so I can't fix it quickly and send the patch to the author. Hopefully he'll check into this thread soon and come up with a fix.

  6. Fabian
    Member
    Plugin Author

    Posted 2 years ago #

    Thank you for the detailed report!
    About the file list: I must admit that lists with more than one category never worked correctly. The if($n > $num) break; was to limit the list length, i.e pagination. And now I recall why this was tagged TODO!!, think about $num=0 should be no limit ;). In the meantime the code of ListTpl changed a lot and all existing bugs should be fixed (never said that there aren't any new bugs).

    2nd bug:
    last_parent is used for caching in Item::GetParent. I replicated the issue, and fixed it by just removing the ancestor check in NotifyFileRemoved. This should be safe, since this function is always called on a file's or category's parent.

    Update will be released during this week.

  7. Fabian
    Member
    Plugin Author

    Posted 2 years ago #

    Btw, do a File Sync to fix the file counters.

  8. ccalvert
    Member
    Posted 2 years ago #

    Thanks for your quick reply. With regard to the detailed bug report, you're welcome. As a programmer, I find vague bug reports irritating and try not to do that to others. :) I'll keep an eye out for the update.

  9. ccalvert
    Member
    Posted 2 years ago #

    FYI, the file sync fixed the counts just as you said. Thanks.

Topic Closed

This topic has been closed to new replies.

About this Plugin

About this Topic