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

Footnote make Page and UUID unique together #16

Merged
merged 4 commits into from
Jun 30, 2021

Conversation

CamLamb
Copy link

@CamLamb CamLamb commented Jun 24, 2021

No description provided.

@CamLamb
Copy link
Author

CamLamb commented Jun 24, 2021

TODO:

  • Add validation to check that UUIDs stored in the page reference related footnotes that exist

@CamLamb
Copy link
Author

CamLamb commented Jun 24, 2021

Current thoughts on validation:

I was thinking something like this on the RichTextBlockWithFootnotes class:

class RichTextBlockWithFootnotes(RichTextBlock):
    ...
    def clean(self, value):
        cleaned_data = super().clean(value)
        page = {} # Not sure how to get the page from here...
        footnote_uuids = []

        def add_uuid(match):
            footnote_uuids.append(match.group(1))
        FIND_FOOTNOTE_TAG.sub(add_uuid, str(value))

        for footnote_uuid in footnote_uuids:
            if not page.footnotes.filter(uuid=footnote_uuid).exists():
                raise ValidationError(f"Invalid footnote reference {footnote_uuid}")
        return cleaned_data

but I couldn't work out how to get the page (I'm not convinced this is possible).

Open to suggestions/ideas

@CamLamb CamLamb force-pushed the feature/page-uuid-unique-together branch from 66e88e8 to 7d92d2b Compare June 25, 2021 10:06
@CamLamb
Copy link
Author

CamLamb commented Jun 25, 2021

I am splitting the Validation logic out into its own issue to be dealt with separately as I think it needs a little more thought.
See: #17

@CamLamb CamLamb linked an issue Jun 25, 2021 that may be closed by this pull request
Cameron Lamb added 4 commits June 25, 2021 11:54
We no longer run .get(), instead we store a list of the page's footnotes, we will get a KeyError if there is an invalid footnote reference.
@CamLamb CamLamb force-pushed the feature/page-uuid-unique-together branch from 7d92d2b to 8100fe8 Compare June 25, 2021 11:01
@CamLamb CamLamb merged commit f0685d3 into master Jun 30, 2021
@CamLamb CamLamb deleted the feature/page-uuid-unique-together branch June 30, 2021 13:06
jhonatan-lopes added a commit to MozillaFoundation/wagtail-footnotes that referenced this pull request Jan 16, 2024
* Cleanup RichTextBlockWithFootnotes (#2)

* Cleanup RichTextBlockWithFootnotes

* Make sure we have a context to work from and add a comment

* Remove unnecessary step from readme

* Add footnotes as a feature when using RichTextBlockWithFootnotes

* Update changelog

* Fix replace_footnote_tags by passing 'value' through and correct features

* Footnote make Page and UUID unique together (torchbox#16)

* Make Footnotes unique on page-uuid

* Add migration

* Fix migration and Footnote.DoesNotExist exception

* Footnote.DoesNotExist is no longer needed

We no longer run .get(), instead we store a list of the page's footnotes, we will get a KeyError if there is an invalid footnote reference.

* Bump version to 0.7.0

* Only attempt to add footnotes if the context's 'page' is a Page object

* Update wagtail_footnotes/blocks.py

Co-authored-by: Dan Braghis <[email protected]>

* Cast ``Footnote.uuid`` to string so previews work

The keys in ``self.footnotes`` need to be strings in order for lookups to work, but before the ``Footnote`` is saved to the db, it is of type ``UUID``.

Refs torchbox#23

* update wagtail 3.0 code changes

* update package information

* update CHANGELOG

* Bump version and update changelog

* Fix typo

* Modify setup.py so package can be detected by GitHub (torchbox#31)

* Bump to version 0.8.0

* Drop Django constraint and let Wagtail dictate it

* Adds apps file to specifcy auto field

* Add GitHub Action to publish to PyPI on release

* Allow wagtail 4

* Alter footnotes admin css selector

* Adjust footnotes template to match the new style

* Wagtail 2.15,2.16 & 3.0 adjustments

They need an alternative template file and javascript file

* Fixed typo in template name (torchbox#44)

* Revise version to 0.9.0

* Update CHANGELOG.md

* Update CHANGELOG.md

* Updates to support Wagtail 5 (torchbox#49)

* Drop support for Wagtail <  4.1
* Drop support for Python 3.7
* Update pre-commit hooks
* Add tests
* Add support for Wagtail 5+

* Switch to using ruff

* Lint

* Remove unused template/JS

* Tidy up GitHub Actions

* Switch to tox 4

* Test against Wagtail 5.1 (and drop 4.2)

* Tidy up coverage configuration

* Build and use wheel in CI

* Add coverage report in CI

* Switch to using PyPI trusted publishing (torchbox#53)

* Switch to using flit for package build (torchbox#54)

* Drop the docs folder

these are draft notes and are not up ready yet

* Update the CHANGELOG

* Bump version to 0.10.0

* Added basic contributing notes (torchbox#56)

Refs torchbox#55

* Add formal Wagtail 5.2 support (torchbox#63)

* Add Wagtail 5.2 and Python 3.12 in test matrices, update CHANGELOG.md
* Wagtail 5.2 upgrade consideration: Block.get_template now accepts a value argument
* Remove instances of Wagtail 5.0 in the test matrix, Update README.md
---------

Co-authored-by: Katherine Domingo <[email protected]>

* Adapt syntax for Wagtail 4

* Reorder migration

---------

Co-authored-by: Cameron Lamb <[email protected]>
Co-authored-by: Alex Bridge <[email protected]>
Co-authored-by: Dan Braghis <[email protected]>
Co-authored-by: John-Scott Atlakson <[email protected]>
Co-authored-by: Brian Xu <[email protected]>
Co-authored-by: Kevin <[email protected]>
Co-authored-by: Kevin <[email protected]>
Co-authored-by: Oliver Parker <[email protected]>
Co-authored-by: Nick Moreton <[email protected]>
Co-authored-by: nick.moreton <[email protected]>
Co-authored-by: zerolab <[email protected]>
Co-authored-by: Kat <[email protected]>
Co-authored-by: Katherine Domingo <[email protected]>
jhonatan-lopes added a commit to MozillaFoundation/wagtail-footnotes that referenced this pull request Jan 16, 2024
* Cleanup RichTextBlockWithFootnotes (#2)

* Cleanup RichTextBlockWithFootnotes

* Make sure we have a context to work from and add a comment

* Remove unnecessary step from readme

* Add footnotes as a feature when using RichTextBlockWithFootnotes

* Update changelog

* Fix replace_footnote_tags by passing 'value' through and correct features

* Footnote make Page and UUID unique together (torchbox#16)

* Make Footnotes unique on page-uuid

* Add migration

* Fix migration and Footnote.DoesNotExist exception

* Footnote.DoesNotExist is no longer needed

We no longer run .get(), instead we store a list of the page's footnotes, we will get a KeyError if there is an invalid footnote reference.

* Bump version to 0.7.0

* Only attempt to add footnotes if the context's 'page' is a Page object

* Update wagtail_footnotes/blocks.py

Co-authored-by: Dan Braghis <[email protected]>

* Cast ``Footnote.uuid`` to string so previews work

The keys in ``self.footnotes`` need to be strings in order for lookups to work, but before the ``Footnote`` is saved to the db, it is of type ``UUID``.

Refs torchbox#23

* update wagtail 3.0 code changes

* update package information

* update CHANGELOG

* Bump version and update changelog

* Fix typo

* Modify setup.py so package can be detected by GitHub (torchbox#31)

* Bump to version 0.8.0

* Drop Django constraint and let Wagtail dictate it

* Adds apps file to specifcy auto field

* Add GitHub Action to publish to PyPI on release

* Allow wagtail 4

* Alter footnotes admin css selector

* Adjust footnotes template to match the new style

* Wagtail 2.15,2.16 & 3.0 adjustments

They need an alternative template file and javascript file

* Fixed typo in template name (torchbox#44)

* Revise version to 0.9.0

* Update CHANGELOG.md

* Update CHANGELOG.md

* Updates to support Wagtail 5 (torchbox#49)

* Drop support for Wagtail <  4.1
* Drop support for Python 3.7
* Update pre-commit hooks
* Add tests
* Add support for Wagtail 5+

* Switch to using ruff

* Lint

* Remove unused template/JS

* Tidy up GitHub Actions

* Switch to tox 4

* Test against Wagtail 5.1 (and drop 4.2)

* Tidy up coverage configuration

* Build and use wheel in CI

* Add coverage report in CI

* Switch to using PyPI trusted publishing (torchbox#53)

* Switch to using flit for package build (torchbox#54)

* Drop the docs folder

these are draft notes and are not up ready yet

* Update the CHANGELOG

* Bump version to 0.10.0

* Added basic contributing notes (torchbox#56)

Refs torchbox#55

* Add formal Wagtail 5.2 support (torchbox#63)

* Add Wagtail 5.2 and Python 3.12 in test matrices, update CHANGELOG.md
* Wagtail 5.2 upgrade consideration: Block.get_template now accepts a value argument
* Remove instances of Wagtail 5.0 in the test matrix, Update README.md
---------

Co-authored-by: Katherine Domingo <[email protected]>

* Adapt syntax for Wagtail 4

* Reorder migration

---------

Co-authored-by: Cameron Lamb <[email protected]>
Co-authored-by: Alex Bridge <[email protected]>
Co-authored-by: Dan Braghis <[email protected]>
Co-authored-by: John-Scott Atlakson <[email protected]>
Co-authored-by: Brian Xu <[email protected]>
Co-authored-by: Kevin <[email protected]>
Co-authored-by: Kevin <[email protected]>
Co-authored-by: Oliver Parker <[email protected]>
Co-authored-by: Nick Moreton <[email protected]>
Co-authored-by: nick.moreton <[email protected]>
Co-authored-by: zerolab <[email protected]>
Co-authored-by: Kat <[email protected]>
Co-authored-by: Katherine Domingo <[email protected]>
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.

502 error if a footnote call points to a missing footnote
2 participants