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

Implement relationships & tweaks to improve Django admin #74

Open
wants to merge 16 commits into
base: rc/0.2.1
Choose a base branch
from

Conversation

EvdH0
Copy link
Contributor

@EvdH0 EvdH0 commented Mar 20, 2022

The pull request includes a proposal to add functionality to add or remove relationships between nodes. Many-to-Many relationships show up as a multiple select elements (and the horizontal/vertical filter widgets can be applied in the Django admin). Additionally minor improvements to the Django admin experience are included.

Features

  • Add relationships as editable fields that work in the Django admin by using the Django App Registry
  • Possibility of adding horizontal/vertical filter widgets to the relationships in the Django admin
  • Delete button in the Django admin did not work, fixed
  • Changed the pk property into an alias instead of a reference to the primary-key (because the Django internals do not address pk consistently in the same way). This allows general lookup (and for example fixes the delete button in Django admin)
  • Date widget work now in the Django admin
  • Expanded the test suite with few more cases

Limitations

  • 'Multi select delete' actions in Django overview admin still doesn't work yet
  • For DateFields a widget is shown in the admin, however for DateTime fields this widget is not shown but just a normal input box
  • Cardinality constraints on relationships are not properly handled, currently works well for ZeroOrMore, but not for One. A relationship is currently also defined as a ManyToMany field, which is not correct if it would be a ZeroOrOne (would be a ForeignKey)
  • Properties on relationships cannot be edited yet
  • Choices work, but if an empty option is needed this needs to be defined in the choices of the constructor explicitly (ie STATUS = {'':'Empty','F': 'Female', 'M': 'Male', 'O': 'Other'}. Need to check if this is the intended behaviour in neomodel

@werneckpaiva
Copy link

werneckpaiva commented Sep 2, 2023

Any plans to accept this pull request?

@mariusconjeaud mariusconjeaud changed the base branch from master to rc/0.1.2 May 21, 2024 14:34
@mariusconjeaud
Copy link
Collaborator

mariusconjeaud commented May 21, 2024

I am looking at it now @EvdH0 @werneckpaiva. As they say, better late than never. Even when very mega insanely late ?

neomodel 5.3.0 moved some internals
@mariusconjeaud
Copy link
Collaborator

mariusconjeaud commented May 21, 2024

Tests fail for Python 3.10 and above with : AttributeError: 'NeoNodeSet' object has no attribute '_prefetch_related_lookups'

Looks like it is because the currently used version of Django is too old. I'll see if I can upgrade.

Fixed things ; the problem was actually that the modifications introduced indirect calls to the prefetch mechanism, which was not implemented in django-neomodel.
Added a "passthrough" implementation, just to prevent it from failing. It would be interesting to actually implement it though.

@mariusconjeaud mariusconjeaud deleted the branch neo4j-contrib:rc/0.2.1 May 22, 2024 07:21
@mariusconjeaud mariusconjeaud changed the base branch from rc/0.1.2 to rc/0.2.0 May 22, 2024 07:28
@mariusconjeaud
Copy link
Collaborator

Tests now pass ; but if you open the admin interface and try to list / add some Book / Shelf / Author (so the classes that are modified by this), then it fails.

@mariusconjeaud mariusconjeaud changed the base branch from rc/0.2.0 to rc/0.2.1 May 22, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants