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

fix deletion of entity objects / GenericEntitiesDeleteView #458

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

koeaw
Copy link
Contributor

@koeaw koeaw commented Dec 6, 2023

No description provided.

@koeaw koeaw force-pushed the kk/fix/delete_view branch 2 times, most recently from 2c8d3e2 to 875f597 Compare December 6, 2023 12:25
@koeaw koeaw force-pushed the kk/fix/delete_view branch 3 times, most recently from 39feb04 to c19ad69 Compare December 12, 2023 14:12
@koeaw
Copy link
Contributor Author

koeaw commented Dec 13, 2023

Update on why this is still a draft:

  • After last week's discussion at the JF, which also touched on URLs, I realised I never had another look at my urlpatterns, so was wondering if that's what's to blame for some of what I'm (not) seeing
  • I guess ideally, I wouldn't create a separate deletion confirmation for entities, even though I find it more intuitive to have them in one place (I didn't immediately find the shared one)... so I'd need to invest more time to figure out if/where one for entities differs from a more generic one, and either edit the existing one or create one that extends the shared on

@koeaw koeaw linked an issue Dec 20, 2023 that may be closed by this pull request
@koeaw koeaw force-pushed the kk/fix/delete_view branch from c19ad69 to e84a2f3 Compare December 20, 2023 17:24
@koeaw koeaw marked this pull request as ready for review December 20, 2023 17:25
@koeaw
Copy link
Contributor Author

koeaw commented Dec 20, 2023

Reworked to solve only the most pressing issue, see this comment.

Interestingly, this actually shows the object name in the confirmation template for me now. 🤔

@koeaw koeaw requested a review from b1rger December 20, 2023 17:28
@koeaw koeaw changed the title Fix GenericEntitiesDeleteView, add entity-specific template fix deletion of entity objects / GenericEntitiesDeleteView Dec 20, 2023
Copy link
Contributor

@b1rger b1rger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shows the __str__ of the object being deleted, seen as a RootObject - RootObject.__str__ returns RootObject.name. This works, but we are also planning on getting rid of RootObject.name. If we instead use get_queryset instead of setting the model:

def get_queryset(self):
   return self.entity_model.objects.all()

it should instead use the __str__ of the specific entity (i.e. if the Person entity overrides the __str__ (i.e. to include first_name) it uses that one) - could you try that?

@koeaw
Copy link
Contributor Author

koeaw commented Dec 20, 2023

Ahh, right!

Sorry, forgot that we discussed this (I think), will change it.

In GenericEntitiesDeleteView, override get_queryset() method
instead of referencing a specific (parent) model to get at the
object-to-be-deleted.

Closes #485
@koeaw koeaw force-pushed the kk/fix/delete_view branch from e84a2f3 to 87c50f1 Compare December 20, 2023 18:41
@b1rger b1rger merged commit e95e030 into main Dec 21, 2023
7 checks passed
@b1rger
Copy link
Contributor

b1rger commented Dec 21, 2023

Thanks!

@koeaw koeaw deleted the kk/fix/delete_view branch September 30, 2024 12:19
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.

DeleteView doesn't work for projects which don't use TempEntityClass
2 participants