City-of-Helsinki / wagtail-svgmap

ImageMap functionality for Wagtail through inline SVGs
BSD 3-Clause "New" or "Revised" License
13 stars 8 forks source link

Wagtail 2 #13

Closed stuaxo closed 6 years ago

stuaxo commented 6 years ago

Any chance of wagtail 2 support?

Most apps seem to do this by wrapping all the imports with stuff like wagtailcore, e.g.

try:
    import wagtail.core.foo
except ImportError:
    import wagtail.wagtailcore.foo
juyrjola commented 6 years ago

Would be great! PR much appreciated. =)

stuaxo commented 6 years ago

Any idea how I've broken these unit tests?

https://travis-ci.org/City-of-Helsinki/wagtail-svgmap/jobs/338979401

Probably something silly I've done, but might be something more easily fixed by someone that knows the codebase ?

akx commented 6 years ago

Since dependencies haven't been locked (which is a good idea for a library like this), it's just "test rot" as dependencies have changed between the last successful test and this.

Among others (which I elided here for brevity), these have updated:

Django 1.10.2 -> 1.11.10 django-modelcluster 2.0 -> 3.1 django-taggit 0.18.3 -> 0.22.2 django-treebeard 4.0.1 -> 4.2.0 djangorestframework 3.4.7 -> 3.6.4 Pillow 3.4.1 -> 5.0.0 wagtail 1.6.3 -> 1.13.1 Willow-0.3.1 -> Willow 1.0

The test_page_creation test with this

>       assert example_map_option.get('selected')
E       assert ''
E        +  where '' = <bound method Tag.get of <option selected="" value="1">Example Map ZsbCrN2Ir3Sm</option>>('selected')
E        +    where <bound method Tag.get of <option selected="" value="1">Example Map ZsbCrN2Ir3Sm</option>> = <option selected="" value="1">Example Map ZsbCrN2Ir3Sm</option>.get

failure could be that Wagtail or Django has changed the way selected gets serialized in the admin. The simple fix is likely to just change assert example_map_option.get('selected') to assert example_map_option.get('selected') is not None.

The second failure has to do with what the view's context contains. It's obviously used to contain form at some point, but it might be something else like delete_form with the new package versions.

akx commented 6 years ago

@stuaxo I fixed those issues in #15, so if you rebase #14 on top of master, you should be able to continue.

Also, I added Wagtail version variations to .travis.yml, which you should be able to use to also test against Wagtail 2.0 (beta?).

stuaxo commented 6 years ago

Cheers, will do.

The wagtail version is the newest 2.x on pypi, currently wagtail==2.0b1

Since they recommend projects to be on 2, I'd expect a non beta release relatively soon.

juyrjola commented 6 years ago

Thanks a lot, @akx!

stuaxo commented 6 years ago

Oops, merged, not rebased, seemed to get away with it as tests pass now :)

The only complication is sorting travis - since wagtail doesn't support python 2.7.

akx commented 6 years ago

Set up a build matrix exclude clause. For instance if the WAGTAIL envvar is wagtail==2.0b1, add that to the env list and then an exclude stanza like this:

matrix:
  exclude:
  - python: 2.7
    env: WAGTAIL=WAGTAIL==2.0b1
stuaxo commented 6 years ago

(excuse the scrappy commit history, squash these if they get merged :)

I managed to really confuse myself about build matrices - it still seems to try and build with python 2.7 on my current .travis -

https://github.com/stuaxo/wagtail-svgmap/blob/821416843fba340afd3c87365fa09d22c9e0ce87/.travis.yml

stuaxo commented 6 years ago

OK, commits all squashed and got everything working in the wsm_test dir.

Didn't manage to work out the excludes thing, but I'll leave that for now.

In wsm_test:

Possible breaking change is I removed the session auth middleware - it doesn't exist in 2.0 and is implicit in 1.10.

Newer wagtail needs on_delete on Foreign keys, so I set this to SET_NULL

stuaxo commented 6 years ago

Closed as PR was merged.