Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
  • castellum castellum
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 2
    • Issues 2
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 1
    • Merge requests 1
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Releases
  • Packages and registries
    • Packages and registries
    • Container Registry
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • CastellumCastellum
  • castellumcastellum
  • Merge requests
  • !1533

1766 showup in list

  • Review changes

  • Download
  • Email patches
  • Plain diff
Closed Bengfort requested to merge 1766-show-up-in-list into main Sep 14, 2020
  • Overview 3
  • Commits 5
  • Pipelines 3
  • Changes 10

So the was supposed to be a simple template change to display showup in the execution list. However, I fell down the annotate/property rabbit hole.

The simple way to add a computed field to a model is by defining a @property. This works well and is easy to understand. However, it becomes a performance issue on lists if the property makes additional database requests.

The better option in that case is to calculate the field in the database using .annotate(). That is more complicated but can improve performance a lot.

The big trouble with .annotate() is that it is not available everywhere, notably instances that have not yet been saved to the database.

Another issue is that you have to decide: Either you add the annotation to a model's default manager which makes every single query more complicated, or you do the annotation on a case-by-case basis, which makes them unavailable in many situations.

The holy grail would be to combine both approaches: To have a single definitions that works and is efficient in all cases.

This MR could bring us a step closer to that goal: It adds the annotate_instance() helper that will get annotations from the database and add them to an existing instance, but only if they do not exist yet. It is based on this stackoverflow answer.

This helper is then used to efficiently render showup both on single subjects as well as on a pre-annotated list of subjects.

Scratch everything I wrote above. The list we need to show the showup in is not actually a list of subjects, but a list of participations. So the issue is that we cannot annotate a .select_related() query -- at least I don't know how. [.prefetch_related() did the trick]

So not the big question is: Do we want this?

  • Merge like this
  • Remove the optimization and just use @property instead
  • Do not show showup in the list at all
Edited Sep 14, 2020 by Bengfort
Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: 1766-show-up-in-list