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

content overview: add tabs for models and display all objects ordered by date created #908

Merged
merged 3 commits into from
Aug 2, 2023

Conversation

lucasmarchd01
Copy link
Contributor

@lucasmarchd01 lucasmarchd01 commented Aug 1, 2023

In this PR, modifications are made to the content-overview page. Previously, this page would only display the last 50 updated objects in our database. The way this information was fetched was inefficient, making it impossible to scale to use all the objects in our database without affecting the page loading time. The solution implements tabs at the top of the page that represent the relevant models in our database. When the user clicks on one of these tabs, we will display all of the objects corresponding to this model ordered by "-date_created". The user can now paginate through all of these objects with 100 objects per page. Fixes #896

image

Copy link
Contributor

@jacobdgm jacobdgm left a comment

Choose a reason for hiding this comment

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

This looks good! I left one comment to check that Century models haven't been overlooked, and one suggesting a possible simplification to the code.

One other thing: do any tests need to be updated to match these changes? If not, maybe now is a good time to write a good set of tests for this view.

<td class="text-wrap" style="text-align:center">
<a href="{{ object.get_absolute_url }}"><b>{{ object.full_name }}</b></a>
{% if object.last_updated_by is None %}
{{ object.last_updated_by }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be clearer to just put None rather than {{ object.last_updated_by }} (since we already know it's None)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense!

@lucasmarchd01
Copy link
Contributor Author

lucasmarchd01 commented Aug 1, 2023

One other thing: do any tests need to be updated to match these changes? If not, maybe now is a good time to write a
good set of tests for this view.

Thanks for the suggestion, will do.

@jacobdgm jacobdgm merged commit a65efad into DDMAL:develop Aug 2, 2023
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.

Content-Overview page - we should paginate results
2 participants