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

Remove __str__ methods from models #234

Merged
merged 5 commits into from
May 19, 2024
Merged

Conversation

meshy
Copy link
Contributor

@meshy meshy commented May 19, 2024

Before this change, we relied on a string representation of the models in the UI, but display logic should not belong on models.

We now build the display information outside the models as required.

These methods might still be useful for debugging, but as we're on a journey to remove the models altogether, I don't think they're worth keeping.

meshy added 5 commits May 19, 2024 15:58
When comparing model objects, Django will compare only the ID of the
models. As we are moving away from using models in templates, we should
compare against something more sensible: the module name.
Before this change, we relied on a string representation of the
ProjectVersion model in each of these places, but display logic should
not belong on models.

We now build the required display information outside of the
ProjectVersion model.
Before this change, we relied on a string representation of the
Module model, but display logic should not belong on models.

We only needed to get the module name in one place, and use it in two.

We now pass a string to the template instead of a whole model.
Before this change, we relied on a string representation of the
Klass model, but display logic should not belong on models.

We now use the required field directly in templates instead.
We don't ever rely on these representations directly in the UI.
@meshy meshy merged commit 765ae15 into main May 19, 2024
1 check passed
@meshy meshy deleted the remove-model-__str__-methods branch May 19, 2024 15:11
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.

1 participant