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

[Bug 1401246] Fixing select_related to select only foreignkey fields #4804

Merged
merged 3 commits into from May 22, 2018

Conversation

Projects
None yet
2 participants
@safwanrahman
Contributor

safwanrahman commented May 21, 2018

Fixing all the select_related queryset to get compataibility with django 1.9+.
It seems to be a bug in django code that let selecting non related fields by select_related as it was fetching the whole object instead of the fields only.

The actual use case would be using only for limiting fetch fields instead of full object. In all of our use case, it seems that we are covered with only.

@jwhitlock @escattone r?

@jwhitlock

@safwanrahman thanks for tackling this. TravisCI identified some issues with the PR, noted here.

@@ -241,16 +241,18 @@ def _get_doc_and_fallback_reason(document_locale, document_slug):
doc = None
fallback_reason = None
# Optimizing the queryset to fetch the required values only
related_fields = ['current_revision', 'current_revision__creator',
'parent', 'parent__current_revision', 'parent__topic']

This comment has been minimized.

@jwhitlock

jwhitlock May 22, 2018

Member

There is an extra underscore, parent__topic should be parent_topic ("Web" is the parent topic for "Web/JS")

This comment has been minimized.

@safwanrahman

safwanrahman May 22, 2018

Contributor

Oh! Sorry, my bad!

fields = (['html', 'rendered_html', 'zone_subnav_local_html', 'body_html',
'locale', 'slug', 'title', 'is_localizable', 'rendered_errors',
'toc_html', 'summary_html', 'summary_text', 'quick_links_html']
+ current_revision_fields + parent_fields + parent_topic_fields)

This comment has been minimized.

@jwhitlock

jwhitlock May 22, 2018

Member

flake8 requests that the + sign ends the previous line rather than starts this line.

This comment has been minimized.

@safwanrahman

safwanrahman May 22, 2018

Contributor

having the + sign at previous line makes me confuse while I read the code.
So for readibility, I have made another field to make flake8 happy

@safwanrahman

This comment has been minimized.

Contributor

safwanrahman commented May 22, 2018

@jwhitlock Test passed! r?

@jwhitlock

Looks good, thanks @safwanrahman!

Take a look at the guidelines for branches, commit messages, etc. for the next PR, please:

https://github.com/mozilla/kuma/blob/master/CONTRIBUTING.md#conventions

@jwhitlock jwhitlock merged commit f21acef into mozilla:master May 22, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
security/snyk - package.json No dependency changes
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment