thclark / wagtail_events

Events calendar management for wagtail
Other
10 stars 4 forks source link

TypeError with EventIndex live page #19

Open sebastienhasa opened 3 years ago

sebastienhasa commented 3 years ago

Hello.

If I install wagtail-events, create and publish an event index page then try to see the live version of this page, I got a TypeError exception:

QuerySet indices must be integers or slices, not str.

Request Method:     GET
Request URL:    http://127.0.0.1:8000/agenda/
Django Version:     3.0.8
Exception Type:     TypeError
Exception Value:    

QuerySet indices must be integers or slices, not str.

Exception Location:     /home/seb/.virtualenvs/w/lib/python3.8/site-packages/django/db/models/query.py in __getitem__, line 286
Python Executable:  /home/seb/.virtualenvs/w/bin/python
Python Version:     3.8.5
Python Path:    

['/home/seb/tmp/Wagtail/test',
 '/home/seb/.virtualenvs/w/lib64/python38.zip',
 '/home/seb/.virtualenvs/w/lib64/python3.8',
 '/home/seb/.virtualenvs/w/lib64/python3.8/lib-dynload',
 '/usr/lib64/python3.8',
 '/usr/lib/python3.8',
 '/home/seb/.virtualenvs/w/lib/python3.8/site-packages']

Server time:    Fri, 7 Aug 2020 15:56:15 +0000

Wagtail-events version:

0.4.0

thclark commented 3 years ago

Have you got a better stacktrace that that, @sebastienhasa ? There's not enough information to reproduce this and a full stacktrace might point to where the issue is.

sebastienhasa commented 3 years ago
Internal Server Error: /agenda/
Traceback (most recent call last):
  File "/home/seb/.virtualenvs/w/lib/python3.8/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/home/seb/.virtualenvs/w/lib/python3.8/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/home/seb/.virtualenvs/w/lib/python3.8/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/seb/.virtualenvs/w/lib/python3.8/site-packages/wagtail/core/views.py", line 24, in serve
    return page.serve(request, *args, **kwargs)
  File "/home/seb/.virtualenvs/w/lib/python3.8/site-packages/wagtail/core/models.py", line 771, in serve
    self.get_context(request, *args, **kwargs)
  File "/home/seb/.virtualenvs/w/lib/python3.8/site-packages/wagtail_events/abstract_models.py", line 114, in get_context
    queryset['items'],
  File "/home/seb/.virtualenvs/w/lib/python3.8/site-packages/django/db/models/query.py", line 286, in __getitem__
    raise TypeError(
TypeError: QuerySet indices must be integers or slices, not str.
[07/Aug/2020 16:51:31] "GET /agenda/ HTTP/1.1" 500 91823
thclark commented 3 years ago

Right, so I did some investigation and devops work in #21 to try and find this.

I upgraded a bunch of dependencies to try and reproduce this, thinking it might be a Django3.0 issue, but tests are passing (admittedly with not superb coverage) on the new test matrix.

The root of this is some context code that was left in as a hangover from the old and unmaintained library that I forked this from.

The original code has clearly done something custom with the pagination, that I'd never come across because I'm not supporting use with templates (pagination on headless CMS is handled over the APIs).

I suspect you could probably fixit by simplifying some of that pagination code to use the vanilla paginator - the answers here might help. I'll happily accept a PR with the fix if you're interested in contributing, @sebastienhasa ?

thclark commented 3 years ago

Oh, also here's a good guide which should help fix the issue: https://learnwagtail.com/tutorials/how-to-paginate-your-wagtail-pages/

sebastienhasa commented 3 years ago

I'll happily accept a PR with the fix if you're interested in contributing, @sebastienhasa ?

I'm interested. But I cannot do it with this Github account as it's my work account.

On my free time I build a website and I plan to use wagtail_events on it. So when I fix this problem, I can send you the pull request. But I plan to do it in a few weeks.

As I didn't have a personal account on Github, can I send you my pull request by e-mail?

thclark commented 3 years ago

Um, I'm not sure how sending a pull request by email works?!

I'd recommend either just using this account (Github user accounts are inherently tied to the individual, so your workplace shouldn't have a reason to complain so long as you're not doing it on work time) or create a second one (they're free) for personal stuff?

sebastienhasa commented 3 years ago

Um, I'm not sure how sending a pull request by email works?!

Really easy: https://thoughtbot.com/blog/send-a-patch-to-someone-using-git-format-patch

;)

sebastienhasa commented 3 years ago

Based on my first look, the problem of this bug come at wagtail_events/abstract_models.py:114.

You use a string to specify an index in the queryset you get from the line 105.

thclark commented 3 years ago

@sebastienhasa thanks for your help. Just to be clear, are you offering to fix this one bug? Or to join as a contributor to maintain templates for a longer period of time (I'll be maintaining the headless side for at least 3 more years, under a contract)?

If the former, I think I'd rather just remove this legacy pagination code entirely, because it doesn't meet the stated aim of the library and I don't want to get trapped into maintaining code I have no use for (I have other OSS projects I feel are significantly more important). You could always just subclass the abstract model to add your own paginator and templates?

If the latter, you'll need a github account, because I'm not inclined to be manually patching the codebase every time you make a template change, that's not a scalable model of software development!

What do you think?

sebastienhasa commented 3 years ago

thanks for your help. Just to be clear, are you offering to fix this one bug?

I wasn't thinking to join as a contributor. Only to fix this bug and provide an example of template.

If the former, I think I'd rather just remove this legacy pagination code entirely

Without the pagination, if someone need it, he/she should create its own event index page. Wich mean it will exist 2 kinds of event index page. Can be confusing for the website writers/editors.

thclark commented 3 years ago

Well, they'd just inherit from AbstractIndexPage without installing the app, to prevent creation of duplicate models.

But you know what, it's a pretty empty app without even the pagination, so I'll take on the maintenance going forward if you'll fix this and provide an example template via a PR here on github that I can merge quickly and easily!

Thanks sebastien!