bookwyrm
bookwyrm copied to clipboard
Switching edition doesn't work
Describe the bug I try and switch edition of a book and get an error message.
To Reproduce Steps to reproduce the behavior:
- I go to https://bookwyrm.social/book/1215675/s/albert-camus-and-the-human-crisis
- 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).
- 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.
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.
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
):
After clicking (notice the link I'm on says 1970
):
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.
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:
So I'm thinking there are two possible things we can do:
- 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. - 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.
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.
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.