[GUI Plugin] Reading List

[ownedbycats]

I will load Calibre in debug and run that the next time I get bug.

I actually did change max_sort_levels back to the default to see if that was the issue. It didn't help but it did make changing views less slow.

[ownedbycats]

OK I did some testing. First I triggered the bug by adding a book to a list and then removing it. For some reason removing triggers the bug more often.

Action Chains: Starting chain: View Column Sort
Action Chains: View Column Sort: starting action No. 1: View Column Sort
DEBUG: sort_order: [['#onreadinglist', True], ['#readorder', True], ['#currentlyreading', True], ['#percentread', False], ['#lastread', False]]
Action Chains: Is db modified by chain: False
Action Chains: chain (View Column Sort) finished in: 0:00:00.078262

For comparison, here's a book that I hadn't touched and thus isn't affected by the bug and sorts normally. It shows the same order. (I'm not sure what true/false mean in this context; both books have a #percentread and #lastread value.)

Action Chains: View Column Sort: starting action No. 1: View Column Sort
DEBUG: sort_order: [['#onreadinglist', True], ['#readorder', True], ['#currentlyreading', True], ['#percentread', False], ['#lastread', False]]
Action Chains: Is db modified by chain: False
Action Chains: chain (View Column Sort) finished in: 0:00:00.078130

[capink]

The sort order is not book specific, it is library specific and has nothing to do with the selected book. The true/false has has to do with the sorting direction (ascending/descending).

[capink]

When you remove the book, the value of #onreadinglist (the first column in the sort order) changes to from "true" to another value "depending on the template you set", so it moves to the bottom.

[ownedbycats]

#onreadinglist is

program: 
	test(field('#readorder'), 'Yes', '')

I think I created it prior to GPM templates becoming a thing

[capink]

It is now as simple as verifying whether the values of any of the columns in the sort order (#onreadinglist ... etc) changes when you remove a book from the reading list.

I do not know your setup and you did not mention the '#readorder' before and whether it is updated by the reading list plugin. As I said before, without having the full data I'm just shooting in the dark, and will not reach anything. My hunch is that the books are moving to bottom to honor the current sort order.

[ownedbycats]

Originally Posted by ownedbycats

Reading Lists is my series-like, and "on reading list" is a composite that returns 'true' when that column is populated - otherwise I'd have to use descending order to keep it at the top of the list.

I did mention it and its purpose (though I made a slight error, it returns 'yes' instead of 'true').

[JimmXinu]

Since you don't use RL's View List, unless you can point to a column that Reading List didn't update correctly, I don't think your issue is with RL itself. Although that doesn't rule out that it might be triggered by RL.

I'm inclined to agree with capink--an unexpected effect from Calibre's sorting.

[ownedbycats]

I did some more testing.

First I thought that Reading List simultaneously changing the series column and the #kobocoll column might've been breaking things. I checked them seperately.

I found I could only replicate the bug reliably when removing from a reading list that had the series-like column set. (I didn't notice this before, but this bug never occurred with my "Send to Device" list. No series column on that.)

To isolate it to the series column and not the composite, I switched my #onreadinglist composite column to instead check for the presence of "To Be Read" in the #kobocoll column. No effect.

But what did help was removing the series column entirely from the View Manager sort order.

Unfortunately this now means that my books are not actually in order anymore; I'm considering dumping the series column entirely. But I'll try creating a new column with a new lookup key; I'm wondering if the old column might've gotten messed up in some manner.

[ownedbycats]

Creating a new column didn't work. Unfortunate.

[chaley]

Originally Posted by ownedbycats

I switched my #onreadinglist composite column to instead check for the presence of "To Be Read" in the #kobocoll column; that didn't help.

But what did help was removing Reading List entirely from the View Manager sort order.

Originally Posted by ownedbycats

Creating a new column didn't work. Unfortunate.

There are potential API ordering problems when changing columns. AFAIK one should a) change all the columns, then b) do a reset_ids() on books that might have changed. Doing a set_field() does not notify the GUI that something might have changed.

I can think of two problems that can arise. 1) If a column is changed after the GUI is notified or without notifying the GUI then the search and sort caches can have incorrect values. 2) If a field changed without a reset of the book then the composite columns for that book might not be recalculated (another book cache issue).

I know nothing about how Reading List does its job. Does it notify the GUI using model.reset() or model.reset_ids() after changing anything? Does it do a cache.clear_caches()?

[ownedbycats]

I think I found the relevant section in action.py. Lines 569-622:

    def remove_books_from_list(self, list_name, book_id_list, refresh_screen=True, display_warnings=True):
        '''
        This method is designed to be called from other plugins
        list_name - must be a valid list name
        book_id_list - should be a list of calibre book ids to be removed
        refresh_screen - indicates whether to refresh the book details displayed in library view
        display_warnings - option to suppress any error/warning dialogs if books already on list
        Returns a tuple of (removed_lids_ist, any_tags_changed)
        '''
        if list_name is None:
            if display_warnings:
                return error_dialog(self.gui, _('Cannot remove from list'),
                                    _('No list name specified'), show=True)
            return None, False

        if refresh_screen:
            previous = self.gui.library_view.currentIndex()
        db = self.gui.current_db

        with self.sync_lock:
            book_ids = cfg.get_book_list(db, list_name)
            id_map = OrderedDict([(book_id, True) for book_id in book_ids])
            removed_ids = []
            for calibre_id in book_id_list:
                if calibre_id in id_map:
                    removed_ids.append(calibre_id)
                    book_ids.remove(calibre_id)

            if not removed_ids:
                if display_warnings:
                    confirm(_('The selected book(s) do not exist on this list'),
                                'reading_list_not_on_list', self.gui)
                return None, False
            cfg.set_book_list(db, list_name, book_ids)

            # Remove tags from the books if necessary
            any_tags_changed = self.apply_tags_to_list(list_name, removed_ids, add=False)
            changed_series_id_list = self.update_series_custom_column(list_name, book_ids)

            if refresh_screen:
                message = _('Removed %d books from your %s list') % (len(removed_ids), list_name)
                self.gui.status_bar.showMessage(message)
                if any_tags_changed:
                    self.gui.tags_view.recount()
                if unicode(self.gui.search.text()).startswith('marked:reading_list_'):
                    self.view_list(list_name)
                else:
                    refresh_book_ids = set(changed_series_id_list).union(set(removed_ids))
                    self.gui.library_view.model().refresh_ids(refresh_book_ids)
                    current = self.gui.library_view.currentIndex()
                    self.gui.library_view.model().current_changed(current, previous)
                return None, False
            else:
                return (removed_ids, any_tags_changed)

And 949 - 987

    def update_series_custom_column(self, list_name, book_ids):
        changed_series_book_ids = []
        db = self.gui.current_db
        list_map = cfg.get_list_info(db, list_name)
        series_column = list_map.get(cfg.KEY_SERIES_COLUMN, cfg.DEFAULT_LIST_VALUES[cfg.KEY_SERIES_COLUMN])
        if not series_column:
            return changed_series_book_ids

        series_name = list_map.get(cfg.KEY_SERIES_NAME, cfg.DEFAULT_LIST_VALUES[cfg.KEY_SERIES_NAME])
        if not series_name:
            series_name = list_name

        custom_columns = db.field_metadata.custom_field_metadata()
        col = custom_columns.get(series_column, None)
        if col is None:
            return
        label = db.field_metadata.key_to_label(series_column)

        # Find all the books currently with this series name:
        query = '#%s:"%s"' % (label, series_name)
        existing_series_book_ids = db.data.search_getting_ids(query, search_restriction='', use_virtual_library=False)

        # Go through all the books on our list and assign the series name/index
        for idx, book_id in enumerate(book_ids):
            series_idx = idx + 1
            existing_series_name = db.get_custom(book_id, label=label, index_is_id=True)
            existing_series_idx = db.get_custom_extra(book_id, label=label, index_is_id=True)
            if series_name != existing_series_name or series_idx != existing_series_idx:
                db.set_custom(book_id, series_name, label=label, commit=False, extra=series_idx)
                changed_series_book_ids.append(book_id)
            if book_id in existing_series_book_ids:
                existing_series_book_ids.remove(book_id)

        # Any books left on the existing series list are no longer on our reading list
        for book_id in existing_series_book_ids:
            db.set_custom(book_id, '', label=label, commit=False, extra=None)
            changed_series_book_ids.append(book_id)
        db.commit()
        return changed_series_book_ids

[JimmXinu]

Originally Posted by chaley

I know nothing about how Reading List does its job. Does it notify the GUI using model.reset() or model.reset_ids() after changing anything? Does it do a cache.clear_caches()?

Not directly. RL calls self.gui.library_view.model().refresh_ids(refresh_ book_ids) (ie, the ids of the books changed) whenever it potentially changes books.

(And gets current from library_view.currentIndex() and calls model().current_changed(current, previous) with a previously saved currentIndex()).

It also sometimes calls self.gui.tags_view.recount(), but not always. I honestly don't know if that's intentional or oversight.

However, one of the places it isn't called is remove_books_from_list(), so let's see if adding tag_views.recount() everywhere refresh_ids() is called corrects the issue with some side-effect.

Test version attached.

[ownedbycats]

Unfortunately, bug still occurs with the test version.

As soon as I'm done with some other stuff, I'm going to see whether it happens in a test library that has all my columns and stuff copied over. If so, I'll attach it.

[ownedbycats]

Created a fresh test library for replication and was able to repeat results even in a portable Calibre with only Reading List and View Manager. So here it is.

Steps to replicate. For this you'll want both Reading List and View Manager installed.

1. Switch to the "On Device" view. This will let you see all the available books and the reading list series column.
2. Remove one of the books from the "To Be Read" list.
3. Press whatever key is assigned for "Re-apply current sort." I think by default it's F5. Alternately just re-apply the current view.
4. The book should end up getting stuck at the bottom of the list.

It remains there even when switching to Default or Fanfiction views (depending on which book you use for testing). It resolves if you remove "Reading List" from the sort.