WIP: Support django-admin-numeric-filter

[hwalinga]

This adds django-admin-numeric-filter to the test app. It does this by adding a pages field to the Books and add it to the admin.

It works, but it just needs some styling.

See #193

[hwalinga]

Seems the CI does not install the deps specified in the pyproject.toml.

[farridav]

Yeah they also need to be in project.toml as tox takes its config from there ...

See the tox config at the bottom...if you have a decent way of avoiding this duplication im all ears :)

Otherwise, I look forward to reviewing this early next week :)

[hwalinga]

Yeah they also need to be in project.toml as tox takes its config from there ...

Eh, not experienced with tox, but can't you just put poetry install in the commands tox executes?

This might give you more pointers: python-poetry/poetry#1745

[hwalinga]

Unsure what this means:

  E         {'django/forms/widgets/input.html': 26} != {'django/forms/widgets/input.html': 21}
  E         {'django/forms/widgets/attrs.html': 32} != {'django/forms/widgets/attrs.html': 27}

[farridav]

It means that those widgets have been rendered out 5 more times than expected.

These assertions help us to ensure we don't accidentally put includes in loops, and cause the page to take ages to render.

[hwalinga]

Ah, so because I changed the admin, there is now more to render, and that failed the tests? Should I increment those than so that the numbers match?

This does look a bit prone to breaking. Every time someone adds something to the admin, those numbers aren't correct anymore.

Also, do you want in this PR an example usage of #244 as well. That is essentially the exact same problem, but just another third party app.

[farridav]

Sure, you can bump the numbers.

It does require maintenance, though it also prevents error, so it's adding value right now

[hwalinga]

Okay, but do I also need to add an example for #244 then?

[farridav]

Sure that's fine, it's mainly to help us verify the 3rd party app support works as expected.

If the demo app gets too full of 3rd party apps, and obscures the jazzmin implementation we will clean it up.

[hwalinga]

Your CI discovered a funny bug:

  >   date_of_death = factory.LazyAttribute(lambda x: x.date_of_birth.replace(year=NOW.year))
  E   ValueError: day is out of range for month

(leap year)

Fix in: #258

[hwalinga]

So this includes now correctly the two third party apps. Now someone needs to look into the CSS to make this look good. I think you have a much better understanding of CSS and bootstrap, but if you don't have time I don't have any problems looking into it myself.

[hwalinga]

@farridav Friendly ping, as this is gathering merge conflicts.

Should I see if I can fix the CSS to support this, or do you want to take a look? Or should we first merge this already, and in a later PR fix the CSS to add support for it?

[farridav]

If you can take a look at the CSS that's a win, I'm a bit overloaded with other projects at the moment, so tend to be looking at the easy wins on the weekends.

I've left some comments on the PR that we will need to address first.

If the styling is currently OK, we can get it into master, and follow up with jazz as needed

[farridav]

Getting this into mainline, as it provides support, and no breaking changes for those not using the 3rd party app.

But let's follow up with some style improvements asap ..