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

verbose_field_name can break on manytomany relationships #716

Closed
JamesRamm opened this issue May 17, 2017 · 13 comments
Closed

verbose_field_name can break on manytomany relationships #716

JamesRamm opened this issue May 17, 2017 · 13 comments
Labels
Milestone

Comments

@JamesRamm
Copy link

JamesRamm commented May 17, 2017

Some change between 1.0.2 and 1.0.3 has meant verbose_field_name can now raise an AttributeError in cases where the related_name on a ManyToManyRel is None, which can happen.

Specifically this line:

names.append(part.related_name.replace('_', ' '))

I suggest changing it to something like:

if part.related_name:
   names.append(part.related_name.replace('_', ' '))
else:
   names.append('None')

which is what you wouldve got from 1.0.2

@carltongibson
Copy link
Owner

@JamesRamm Yep. Seems reasonable.

If you put together a test case and patch I'll push a new release with that.

@carltongibson
Copy link
Owner

This is ref #686 and test_utils.py

@rpkilby
Copy link
Collaborator

rpkilby commented May 17, 2017

@JamesRamm - can you give a simple example of the models. Would this be related to related_name='+'?

@rpkilby
Copy link
Collaborator

rpkilby commented May 17, 2017

Also, names.append('None') would give the old behavior, but it doesn't seem like the desired behavior.

@JamesRamm
Copy link
Author

I think this may be to do with use of a 'through' model on an m2m, but I cannot be 100% sure - still working toward a smallest possible scenario to repeat it.

@g-as
Copy link

g-as commented May 17, 2017

I have this when reversing a ForeignKey, where part is a ManyToOneRel with a None related_name.

@Fingel
Copy link

Fingel commented May 17, 2017

I can confirm this bug. I'm not sure which model in my application is causing it, but I've had to revert to 1.0.2 for now.

@JamesRamm
Copy link
Author

@AGASS007
Yes that is essentially what ours amounts to - our model relationships are more convoluted so it is a bit harder to decipher where it is coming from, but after running it through get_fields you do end up with a ManyToOneRel with None.

Anyway, ending up with related_name as None is valid in django so it would be best to handle gracefully.

@rpkilby

Also, names.append('None') would give the old behavior, but it doesn't seem like the desired behavior.

I agree it may not be desired behaviour although I think it should be handled since it is a valid situation. I guess an ideal solution would be to somehow fallback on some name based on the ForeignKey model? (class.name or something?)

@carltongibson
Copy link
Owner

I'll take a quick patch with test over the perfect solution for this.

@carltongibson
Copy link
Owner

Right, has anyone got a reproduce for me?

@carltongibson
Copy link
Owner

#722 provides a test an workaround for this. Can you give it a run and confirm it resolves your issue. (We can come back for v1.1 to improve behaviour.)

Bonus points if you can explain the failing test case behaviour.

@Fingel
Copy link

Fingel commented May 18, 2017

#722 solves our issue. Nice!

carltongibson pushed a commit that referenced this issue May 19, 2017
* Failing test for #716

* Handle case where related_name is None
carltongibson pushed a commit that referenced this issue May 19, 2017
* Update CHANGES

* Adjust README

* Fixed year in CHANGES (#721)

* Fix for verbose_field_name (#722)

* Failing test for #716

* Handle case where related_name is None

* Update version and changes for 1.0.4
@carltongibson
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants