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.

Sign up for GitHub

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

Jump to bottom

Search analytics improvements #6050

Merged
merged 6 commits into from
Aug 8, 2019
Merged

Search analytics improvements #6050

merged 6 commits into from
Aug 8, 2019

Conversation

dojutsu-user
Copy link
Member

This is a follow up PR from #6019

@dojutsu-user dojutsu-user added the PR: work in progress Pull request is not ready for full review label Aug 8, 2019
@dojutsu-user
Copy link
Member Author

@ericholscher
I have made the suggested changes.

Not sure if we really even want to cascade these deletes. Is there a reason we don't want to store Version here as a string, so we can keep them forever even if a version is deleted? (comment)

We can definitely do these. But we are keeping nothing for forever -- we delete the old queries every day. So I think that won't be of much benefit?

@dojutsu-user dojutsu-user requested review from ericholscher and a team August 8, 2019 16:13
@dojutsu-user
Copy link
Member Author

We do have some queries to optimize if they break in the prod. 🤞

ericholscher
ericholscher approved these changes Aug 8, 2019
View reviewed changes
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Looks much better 👍

readthedocs/projects/views/base.py Outdated Show resolved Hide resolved
readthedocs/templates/projects/projects_search_analytics.html Outdated Show resolved Hide resolved
@ericholscher
Copy link
Member

Looks good, will ship this out today. 👍

@ericholscher ericholscher merged commit df17d9d into readthedocs:master Aug 8, 2019
@dojutsu-user dojutsu-user deleted the search-analytics-improvements branch August 8, 2019 17:39
@dojutsu-user dojutsu-user removed the PR: work in progress Pull request is not ready for full review label Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericholscher ericholscher ericholscher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dojutsu-user @ericholscher