Fixed #33651 -- Added custom queryset support to generic foreign key

[ab-revanthgss]

Modified the prefetch queryset function in generic foreign key field to support custom queryset of different content types.

[github-actions[bot]]

Hello @ab-revanthgss! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

[revanthgss]

@felixxm Thanks for the review👍. I will look into the changes you suggested and update the PR

[ab-revanthgss]

@felixxm Any update on this one?

[ab-revanthgss]

@felixxm Can you review this PR and let me know if there are any changes required?

Thanks!!

[ab-revanthgss]

@David-Wobrock @carltongibson Tagging you to review this code as this PR is inactive since long time.

[carltongibson]

Hi @ab-revanthgss — yes, thanks. This PR is on the review queue — patience is required I'm afraid. (We recently had the 4.1a1 release, which takes a lot of bandwidth in the run up to it.)

[ab-revanthgss]

@carltongibson Cool, no issues👍🏻. I just wanted to know this info.

Thanks

[felixxm]

@ab-revanthgss Do you have time to keep working on this?

[ab-revanthgss]

@ab-revanthgss Do you have time to keep working on this?

Hi @felixxm , Sorry for the delay. I am busy with some other work.

I tried looking into changing queryset argument to querysets. I think one good solution is to have a GenericPrefetch class that subclasses from Prefetch class to support querysets. To make this change, I need to dig deep on how the Prefetch works which is taking time.

Do you have any suggestions on how the querysets argument can be added or do you have some docs on Prefetch internals?

I will try to push the changes this week. In any case, I will revert back to you.

Thanks.

[felixxm]

I tried looking into changing queryset argument to querysets. I think one good solution is to have a GenericPrefetch class that subclasses from Prefetch class to support querysets. To make this change, I need to dig deep on how the Prefetch works which is taking time.

Yes, it's better idea than changing the current implementation of Prefetch. It's exactly what Simon suggested.

[ab-revanthgss]

Hi @felixxm ,

I have added the GenericPrefetch class. Thanks @charettes for suggesting the idea.

Thanks.

[felixxm]

Please take a look on failing tests.

[felixxm]

Closing due to inactivity.

[clement-escolano]

Hello @felixxm @charettes

I am also interested in this feature and would like to offer my help to finish this pull request.

As I understand, the remaining comments ask to:

• move the documentation to django.contrib.contenttypes
• move GenericPrefetch to django.contrib.contenttypes
• deprecate get_prefetch_queryset and replace it with get_prefetch_querysets

Am I missing something?

Cheers

PS : thank you @ab-revanthgss for the initial work, I don't want to take credit for it so if you want to finish this, no worries. Otherwise, I am happy to do it

[smithdc1]

Fab that you are interested in picking this up. 🎁

Your comments seem like reasonable next steps from my reading of the PR.

I'd encourage you to claim the ticket on trac and open a fresh PR when ready for another round of review.

I don't want to take credit for it so if you want to finish this.

We can add multiple authors to recognise contributions from multiple authors.

https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

[ab-revanthgss]

Hi @clement-escolano,

Thanks for picking this up. I initially picked this issue as I felt it's a small change of adding an argument to the existing functionality. But, given that we are thinking of deprecating get_prefetch_queryset(), it's a huge change for me to make as I am contributing for the first time and I am not aware of process of deprecating a feature in Django. Given that I have less time to work on this issue and the complexity of the issue, I feel it will be a faster fix if you work on this.

I am up for taking time and making changes to the PR incase you need any help.

Thanks, Revanth

[clement-escolano]

@smithdc1 Thank you. I will do this, I will start a new PR on Monday then.

@ab-revanthgss No worries, I understand. Thank you again for the initial work, I will try to complete it and I will reach out if I need help.

[clement-escolano]

Hello @smithdc1 I created the pull request here: https://github.com/django/django/pull/17136 It has been several weeks and I had no feedback (except a minor thing from a contributor). Is there something I should do so that the core team is aware of the pull request? I don't want to rush things, I just want to know if I should do something in order for the pull request to be merged or just wait for the core team. Cheers