bookwyrm icon indicating copy to clipboard operation
bookwyrm copied to clipboard

Switching edition doesn't work

Open pivic opened this issue 1 year ago • 5 comments

Describe the bug I try and switch edition of a book and get an error message.

To Reproduce Steps to reproduce the behavior:

  1. I go to https://bookwyrm.social/book/1215675/s/albert-camus-and-the-human-crisis
  2. I see the UI state I have another edition available and I select 'switch to this edition' (the URL to which is https://bookwyrm.social/book/1215676/s/albert-camus-and-the-human-crisis).
  3. The error message 'Server Error: Something went wrong! Sorry about that.' appears on the page https://bookwyrm.social/switch-edition

Expected behavior To be able to switch editions.

Screenshots Screencast of the issue: https://drive.proton.me/urls/VH0EG2VWAM#lxVtv1kf0N8-

Instance BookWyrm.social


This occurs both via desktop and mobile.

pivic avatar Dec 25 '23 10:12 pivic

Same here. The error I get on the web container is this one:

OK: /switch-edition
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "bookwyrm_shelfbook_book_id_shelf_id_7e945535_uniq"
DETAIL:  Key (book_id, shelf_id)=(1009, 3) already exists.


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.9/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.9/site-packages/django/contrib/auth/decorators.py", line 21, in _wrapped_view
    return view_func(request, *args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/django/views/decorators/http.py", line 40, in inner
    return func(request, *args, **kwargs)
  File "/usr/local/lib/python3.9/contextlib.py", line 79, in inner
    return func(*args, **kwds)
  File "/app/bookwyrm/views/books/editions.py", line 91, in switch_edition
    models.ShelfBook.objects.create(
  File "/usr/local/lib/python3.9/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 453, in create
    obj.save(force_insert=True, using=self.db)
  File "/app/bookwyrm/models/shelf.py", line 114, in save
    super().save(*args, priority=priority, **kwargs)
  File "/app/bookwyrm/models/activitypub_mixin.py", line 414, in save
    super().save(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/django/db/models/base.py", line 739, in save
    self.save_base(using=using, force_insert=force_insert,
  File "/usr/local/lib/python3.9/site-packages/django/db/models/base.py", line 776, in save_base
    updated = self._save_table(
  File "/usr/local/lib/python3.9/site-packages/django/db/models/base.py", line 881, in _save_table
    results = self._do_insert(cls._base_manager, using, fields, returning_fields, raw)
  File "/usr/local/lib/python3.9/site-packages/django/db/models/base.py", line 919, in _do_insert
    return manager._insert(
  File "/usr/local/lib/python3.9/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 1270, in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
  File "/usr/local/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1416, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python3.9/site-packages/django/db/backends/utils.py", line 66, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/usr/local/lib/python3.9/site-packages/django/db/backends/utils.py", line 75, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/usr/local/lib/python3.9/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python3.9/site-packages/django/db/utils.py", line 90, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/usr/local/lib/python3.9/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.IntegrityError: duplicate key value violates unique constraint "bookwyrm_shelfbook_book_id_shelf_id_7e945535_uniq"
DETAIL:  Key (book_id, shelf_id)=(1009, 3) already exists.

oculos avatar Jan 02 '24 09:01 oculos

Looks like, for some reason, the "Editions" link defaults to the most recent edition no matter what edition you're currently looking at. For example, I'm looking at this edition of "The Pragmatic Programmer" which is id 1965. However, clicking on the Editions link brings me to the editions of book id 1970, giving me the option to switch to book 1965. However, 1965 is already in my library, causing the error.

Before clicking (notice the link I'm on says book 1965): image

After clicking (notice the link I'm on says 1970): image The top book is not the edition on my shelf (despite the site claiming it is). This is book 1970. The book below is 1965. Clicking to change to this book causes an error because it's already on my shelf.

Seems to be an issue with where the edition link is sending you. I'll try to make a fix tonight.

jakejack13 avatar Jan 04 '24 03:01 jakejack13

After a little more digging, looks like the "primary" edition for the editions link (the edition that the edition link will focus on as seen above) defaults to the parent work always. However, since the editions are bound to their parent works, we can't just change the URL from http://localhost:1333/book/1970/editions to http://localhost:1333/book/1965/editions and call it a day. You get this nasty looking page instead: image

So I'm thinking there are two possible things we can do:

  1. Have every edition be its own parent work, meaning we can just go to <book id>/editions, see the list as we intended, and call it a day. This doesn't seem feasible.
  2. Have the editions page query which edition of a given work the user has, then dynamically modify the buttons to allow the user to switch editions only to the editions they do not currently have on their shelf.

Second one definitely seems like the better option but requires more annoying logic in the front end. I'll play around with it and see if I can come up with anything.

jakejack13 avatar Jan 04 '24 03:01 jakejack13

Alright scratch that, it's just an issue with caching. When you switch editions, the browser doesn't invalidate the cached "active shelf" edition. I'm not too familiar with how Django caching works so hopefully someone who knows more can help out. I've been trying to replace the value in the cache but it just won't budge.

jakejack13 avatar Jan 04 '24 04:01 jakejack13

Thanks for the investigation @jakejack13! What's easiest in this case is to simply invalidate the cache, rather than updating it (at least, that's what I've seen other parts of the codebase do).

I submitted #3217 for review, which would hopefully fix the issue.

Thanks again for the investigation work.

dato avatar Jan 13 '24 17:01 dato