search

DjangoAdminHackers / django-linkcheck

An app that will analyze and report on links in any model that you register with it. Links can be bare (urls or image and file fields) or embedded in HTML (linkcheck handles the parsing). It's fairly easy to override methods of the Linkcheck object should you need to do anything more complicated (like generate URLs from slug fields etc).
BSD 3-Clause "New" or "Revised" License
75 stars 26 forks source link

Ignore raw `pre_save` signal #107

Closed timobrembeck closed 3 years ago

timobrembeck commented 3 years ago

This adds support for the raw flag to linkcheck's pre_save signal.

If the model is saved exactly as presented (i.e. when loading a fixture), one should not query/modify other records in the database as the database might not be in a consistent state yet (see documentation).

This is my first contribution to this repository, so I'm not sure how you would want this change to be represented in the tests. But if you give me a hint, I'm willing to add test cases for this patch. Thanks in advance for your feedback and help!

Fixes #106

claudep commented 3 years ago

Hi Timo, thanks for the report and the patch! Do you think you would be able to add a fixture to the tests, containing a model like yours that fails in get_absolute_url?

timobrembeck commented 3 years ago

@claudep I now added a test case which fails for the current master branch and is fixed with my patch applied.

This was much simpler than I thought because there is another problem with fixtures independent from my special get_absolute_url(): The function instance_pre_save() tries to query the previous object from the database, but in the case of fixtures, this object does not yet exists, even though it has an id: https://github.com/DjangoAdminHackers/django-linkcheck/blob/d086a1ef2d8db56c6fe3f4e49f80238b9bcce782/linkcheck/listeners.py#L119 Both problems are fixed if the pre-save method ignores raw signals.

claudep commented 3 years ago

Thanks a lot!