Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix the book edit confirmation template dropping initial data for dates #893

Merged
merged 5 commits into from Apr 8, 2021
Merged

Conversation

ghost
Copy link

@ghost ghost commented Apr 7, 2021

this is a terrible commit message, sorry xd

the template was checking for initial date values in book, but in the confirmation dialogue, it needs to look in form. it's kind of ugly, but i'm not sure there's a better way that doesn't also rework a lot of book editing view. making this a draft pr in case anyone has a suggestion though!

edit: whoops, meant to post the issue first. related: #894

@mouse-reeve
Copy link
Member

oh, this is good! I bet you don't even need to check for book.published_date, it should be available in the form whether or not the book is provided.

@ghost
Copy link
Author

ghost commented Apr 7, 2021

oh, this is good! I bet you don't even need to check for book.published_date, it should be available in the form whether or not the book is provided.

you can, but that creates a fun different issue haha
the confirmation form has the initial data as a string in "%Y-%m-%d" format, but the edit form (and the book) have it as a datetime. the data is only passed as a formatted string with a TemplateResponse, not with a redirect. only thing i can think of is that maybe TemplateResponse reuses the POST data from the request

@mouse-reeve
Copy link
Member

ah that makes sense, insofar as handling dates ever does. Thank you, I think this is ready to go :)

@ghost
Copy link
Author

ghost commented Apr 7, 2021

apparently django uses the form's data, not instance. think i got it for good

@mouse-reeve
Copy link
Member

nice. If you run ./bw-dev black it will auto-correct the python formatting, by the way

@ghost
Copy link
Author

ghost commented Apr 7, 2021

whoops, that issue should've been obvious in hindsight, i'll test locally until i get it working fine

@ghost
Copy link
Author

ghost commented Apr 7, 2021

this looks good to me. had a few errors but i'm fairly certain they're issues with my particular dev environment

@ghost ghost marked this pull request as ready for review April 7, 2021 01:57
@mouse-reeve
Copy link
Member

When I tried to add a book, I selected a date using the built-in datepicker in Brave/chromium, which formats the date as mm/dd/yyy - when I tried to save, I got this error:

Environment:


Request Method: POST
Request URL: http://localhost/create-book

Django Version: 3.1.6
Python Version: 3.9.0
Installed Applications:
['django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'django.contrib.humanize',
 'django_rename_app',
 'bookwyrm',
 'celery']
Installed Middleware:
['django.middleware.security.SecurityMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.locale.LocaleMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'bookwyrm.timezone_middleware.TimezoneMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware']



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/views/generic/base.py", line 70, in view
    return self.dispatch(request, *args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/django/utils/decorators.py", line 43, in _wrapper
    return bound_method(*args, **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/utils/decorators.py", line 43, in _wrapper
    return bound_method(*args, **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/generic/base.py", line 98, in dispatch
    return handler(request, *args, **kwargs)
  File "/app/bookwyrm/views/books.py", line 187, in post
    formcopy["published_date"] = datetime.strptime(
  File "/usr/local/lib/python3.9/_strptime.py", line 568, in _strptime_datetime
    tt, fraction, gmtoff_fraction = _strptime(data_string, format)
  File "/usr/local/lib/python3.9/_strptime.py", line 349, in _strptime
    raise ValueError("time data %r does not match format %r" %

Exception Type: ValueError at /create-book
Exception Value: time data '' does not match format '%Y-%m-%d'

This is what the browser date picker looks like for me:
Screen Shot 2021-04-07 at 9 07 03 AM

python-dateutil is available in bookwyrm, which has pretty robust date parsing comapred to strptime

@ghost
Copy link
Author

ghost commented Apr 7, 2021

i honestly can't believe that's not standardized lol. i tried writing a test to make sure this doesn't regress, but i couldn't get a test case that passed/failed properly

@mouse-reeve
Copy link
Member

beautiful! works great, even with the wildly inconsistent state of time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant