From ce8bb4b693d45e5e33f94cbf5fb381acb8d593ad Mon Sep 17 00:00:00 2001 From: Radek Czajka Date: Fri, 10 Aug 2012 13:36:29 +0200 Subject: [PATCH] Fix child book reimport bugs. --- apps/catalogue/models/book.py | 64 ++++++++----- apps/catalogue/tasks.py | 5 ++ apps/catalogue/test_utils.py | 3 + apps/catalogue/tests/book_import.py | 134 ++++++++++++++++++++++++++++ 4 files changed, 186 insertions(+), 20 deletions(-) diff --git a/apps/catalogue/models/book.py b/apps/catalogue/models/book.py index cdafedff9..a8b342d74 100644 --- a/apps/catalogue/models/book.py +++ b/apps/catalogue/models/book.py @@ -344,12 +344,17 @@ class Book(models.Model): book.tags = set(meta_tags + book_shelves) - book_tag = book.book_tag() - + obsolete_children = set(b for b in book.children.all() if b not in children) for n, child_book in enumerate(children): child_book.parent = book child_book.parent_number = n child_book.save() + # Disown unfaithful children and let them cope on their own. + for child in obsolete_children: + child.parent = None + child.parent_number = 0 + child.save() + tasks.fix_tree_tags.delay(child) # Save XML and HTML files book.xml_file.save('%s.xml' % book.slug, raw_file, save=False) @@ -379,27 +384,46 @@ class Book(models.Model): book.search_index(index_tags=search_index_tags, reuse_index=search_index_reuse) #index_book.delay(book.id, book_info) - book_descendants = list(book.children.all()) - descendants_tags = set() - # add l-tag to descendants and their fragments - while len(book_descendants) > 0: - child_book = book_descendants.pop(0) - descendants_tags.update(child_book.tags) - child_book.tags = list(child_book.tags) + [book_tag] - child_book.save() - for fragment in child_book.fragments.all().iterator(): - fragment.tags = set(list(fragment.tags) + [book_tag]) - book_descendants += list(child_book.children.all()) + tasks.fix_tree_tags.delay(book) + cls.published.send(sender=book) + return book - for tag in descendants_tags: - tasks.touch_tag(tag) + def fix_tree_tags(self): + """Fixes the l-tags on the book's subtree. - # refresh cache - book.reset_tag_counter() - book.reset_theme_counter() + Makes sure that: + * the book has its parents book-tags, + * its fragments have the book's and its parents book-tags, + * runs those for every child book too, + * touches all relevant tags, + * resets tag and theme counter on the book and its ancestry. + """ + def fix_subtree(book, parent_tags): + affected_tags = set(book.tags) + book.tags = list(book.tags.exclude(category='book')) + parent_tags + sub_parent_tags = parent_tags + [book.book_tag()] + for frag in book.fragments.all(): + affected_tags.update(frag.tags) + frag.tags = list(frag.tags.exclude(category='book')) + sub_parent_tags + for child in book.children.all(): + affected_tags.update(fix_subtree(child, sub_parent_tags)) + return affected_tags + + parent_tags = [] + parent = self.parent + while parent is not None: + parent_tags.append(parent.book_tag()) + parent = parent.parent + + affected_tags = fix_subtree(self, parent_tags) + for tag in affected_tags: + tasks.touch_tag(tag) - cls.published.send(sender=book) - return book + book = self + while book is not None: + book.reset_tag_counter() + book.reset_theme_counter() + book = book.parent def related_info(self): """Keeps info about related objects (tags, media) in cache field.""" diff --git a/apps/catalogue/tasks.py b/apps/catalogue/tasks.py index cf6178939..8cba04fd7 100644 --- a/apps/catalogue/tasks.py +++ b/apps/catalogue/tasks.py @@ -18,6 +18,11 @@ def touch_tag(tag): type(tag).objects.filter(pk=tag.pk).update(**update_dict) +@task(ignore_result=True) +def fix_tree_tags(book): + book.fix_tree_tags() + + @task def index_book(book_id, book_info=None): from catalogue.models import Book diff --git a/apps/catalogue/test_utils.py b/apps/catalogue/test_utils.py index 9692b3277..f42818f7e 100644 --- a/apps/catalogue/test_utils.py +++ b/apps/catalogue/test_utils.py @@ -9,6 +9,8 @@ class WLTestCase(TestCase): """ Generic base class for tests. Adds settings freeze and clears MEDIA_ROOT. """ + longMessage = True + def setUp(self): self._MEDIA_ROOT, settings.MEDIA_ROOT = settings.MEDIA_ROOT, tempfile.mkdtemp(prefix='djangotest_') settings.NO_SEARCH_INDEX = settings.NO_BUILD_PDF = settings.NO_BUILD_MOBI = settings.NO_BUILD_EPUB = settings.NO_BUILD_TXT = settings.NO_BUILD_FB2 = True @@ -24,6 +26,7 @@ class WLTestCase(TestCase): settings.MEDIA_ROOT = self._MEDIA_ROOT settings.CACHES = self._CACHES + class PersonStub(object): def __init__(self, first_names, last_name): diff --git a/apps/catalogue/tests/book_import.py b/apps/catalogue/tests/book_import.py index c8689ea39..09d0e1e79 100644 --- a/apps/catalogue/tests/book_import.py +++ b/apps/catalogue/tests/book_import.py @@ -241,6 +241,140 @@ class ChildImportTests(WLTestCase): 'wrong related theme list') +class TreeImportTest(WLTestCase): + def setUp(self): + WLTestCase.setUp(self) + self.child_info = BookInfoStub( + genre='X-Genre', + epoch='X-Epoch', + kind='X-Kind', + author=PersonStub(("Joe",), "Doe"), + **info_args("Child") + ) + self.CHILD_TEXT = """ + + Pies + Ala ma kota + + """ + self.child = models.Book.from_text_and_meta( + ContentFile(self.CHILD_TEXT), self.child_info) + + self.book_info = BookInfoStub( + genre='X-Genre', + epoch='X-Epoch', + kind='X-Kind', + author=PersonStub(("Joe",), "Doe"), + parts=[self.child_info.url], + **info_args("Book") + ) + self.BOOK_TEXT = """""" + self.book = models.Book.from_text_and_meta( + ContentFile(self.BOOK_TEXT), self.book_info) + + self.parent_info = BookInfoStub( + genre='X-Genre', + epoch='X-Epoch', + kind='X-Kind', + author=PersonStub(("Jim",), "Lazy"), + parts=[self.book_info.url], + **info_args("Parent") + ) + self.PARENT_TEXT = """""" + self.parent = models.Book.from_text_and_meta( + ContentFile(self.PARENT_TEXT), self.parent_info) + + def test_ok(self): + self.assertEqual( + list(self.client.get('/katalog/gatunek/x-genre/' + ).context['object_list']), + [self.parent], + u"There should be only parent on common tag page." + ) + pies = models.Tag.objects.get(slug='pies') + self.assertEqual(self.parent.theme_counter, {pies.pk: 1}, + u"There should be child theme in parent theme counter." + ) + epoch = models.Tag.objects.get(slug='x-epoch') + self.assertEqual(epoch.book_count, 1, + u"There should be only parent in common tag's counter." + ) + + def test_child_republish(self): + CHILD_TEXT = """ + + Pies, Kot + Ala ma kota + + """ + models.Book.from_text_and_meta( + ContentFile(CHILD_TEXT), self.child_info, overwrite=True) + self.assertEqual( + list(self.client.get('/katalog/gatunek/x-genre/' + ).context['object_list']), + [self.parent], + u"There should only be parent on common tag page." + ) + pies = models.Tag.objects.get(slug='pies') + kot = models.Tag.objects.get(slug='kot') + self.assertEqual(self.parent.theme_counter, {pies.pk: 1, kot.pk: 1}, + u"There should be child themes in parent theme counter." + ) + epoch = models.Tag.objects.get(slug='x-epoch') + self.assertEqual(epoch.book_count, 1, + u"There should only be parent in common tag's counter." + ) + + def test_book_change_child(self): + second_child_info = BookInfoStub( + genre='X-Genre', + epoch='X-Epoch', + kind='Other-Kind', + author=PersonStub(("Joe",), "Doe"), + **info_args("Second Child") + ) + SECOND_CHILD_TEXT = """ + + Kot + Ala ma kota + + """ + # Import a second child. + second_child = models.Book.from_text_and_meta( + ContentFile(SECOND_CHILD_TEXT), second_child_info) + # The book has only this new child now. + self.book_info.parts = [second_child_info.url] + self.book = models.Book.from_text_and_meta( + ContentFile(self.BOOK_TEXT), self.book_info, overwrite=True) + + self.assertEqual( + set(self.client.get('/katalog/gatunek/x-genre/' + ).context['object_list']), + set([self.parent, self.child]), + u"There should be parent and old child on common tag page." + ) + kot = models.Tag.objects.get(slug='kot') + self.assertEqual(self.parent.theme_counter, {kot.pk: 1}, + u"There should only be new child themes in parent theme counter." + ) + epoch = models.Tag.objects.get(slug='x-epoch') + self.assertEqual(epoch.book_count, 2, + u"There should be parent and old child in common tag's counter." + ) + self.assertEqual( + list(self.client.get('/katalog/lektura/parent/motyw/kot/' + ).context['fragments']), + [second_child.fragments.all()[0]], + u"There should be new child's fragments on parent's theme page." + ) + self.assertEqual( + list(self.client.get('/katalog/lektura/parent/motyw/pies/' + ).context['fragments']), + [], + u"There should be no old child's fragments on parent's theme page." + ) + + class MultilingualBookImportTest(WLTestCase): def setUp(self): WLTestCase.setUp(self) -- 2.20.1