Feature/SCKAN-364 - Add statement preview row to the export + persist statement preview suffix and prefix #404
Conversation
…ctivityStatement model
…w into statement service
…uffix fields from db
…iew related changes
| def get_prefix_for_statement_preview(cs) -> str: | ||
| connectivity_statement_obj = apps.get_model( | ||
| 'composer', 'ConnectivityStatement') | ||
| connectivity_statement = connectivity_statement_obj.objects.get( |
There was a problem hiding this comment.
question: Isn't cs the connectivity statement instance already?
There was a problem hiding this comment.
here yes all the fields we want can be accessed using the cs instance, and hence I updated the code to get it directly from cs instance. thanks for pointing this out!
| def get_suffix_for_statement_preview(cs): | ||
| connectivity_statement_obj = apps.get_model( | ||
| 'composer', 'ConnectivityStatement') | ||
| connectivity_statement = connectivity_statement_obj.objects.get( |
There was a problem hiding this comment.
It is needed here yes @afonsobspinto, because we need the origin.name and get_laterality_description(), both not accessible with the cs instance. However we can get them using connectivity_statement object.
There was a problem hiding this comment.
It's a similar case as - #381 (comment)
There was a problem hiding this comment.
If we have the time, my preference would be to add the necessary methods inline on the migration (or in a migrations helper file). The reasoning being that I don't think that makes sense to to harm the performance of the server that's running all the time, for ease of use in one time, the migration; Besides I would say conceptually it makes more sense to do so, so that the migration is independent of the model evolution over time.
There was a problem hiding this comment.
thank you for the suggestion @afonsobspinto :)
backend/composer/signals.py
Outdated
| @receiver(m2m_changed, sender=ConnectivityStatement.origins.through, dispatch_uid="update_prefix_suffix_for_connectivity_statement_preview") | ||
| @receiver([post_save, post_delete], sender=ConnectivityStatement, dispatch_uid="update_prefix_suffix_for_connectivity_statement_preview") | ||
| def update_prefix_suffix_for_connectivity_statement_preview(sender, instance, action=None, **kwargs): | ||
| if isinstance(instance, ConnectivityStatement): |
There was a problem hiding this comment.
Here's a good case where we can invert the condition. Instead of having all the logic wrapped in a if we can do:
if not isinstance(instance, ConnectivityStatement):
return
*Function logic*
Jira link - https://metacell.atlassian.net/browse/SCKAN-364
NOTE THE BASE branch is
feature/SCKAN-324.Task description
Why does the signal have
updated_fields?-> Because signal can be called for any changes that are not directly related to the fields updated - like the image below. And this logic makes sure - that we update only either when the m2m changes or direct field changes occur.
In Export's Object text: As per 6th acceptance criteria of Card
