rdmorganiser / rdmo

A tool to support the planning, implementation, and organization of research data management.
https://rdmorganiser.github.io
Apache License 2.0
104 stars 49 forks source link

Within Django views, implement conditions with "vectorized" variables #537

Open Zack-83 opened 2 years ago

Zack-83 commented 2 years ago

Description / Beschreibung

The Django-language allows defining conditions based on variables's values or on variables being empty.

That does not work for variables which can accept a vector of values.

Expected behaviour / Erwartetes Verhalten

A syntaxe such as {% if var %} or {% if var.is_not.empty %} or {% if var|length > 0 %} should work correctly even if var is allowed to be a collection.

These conditions should be evaluated as False if the variable is empty and as True if the variable has 1,2,3,... components.

Steps to reproduce / Schritte zum Reproduzieren

{% load view_tags %}
{% get_set 'project/dataset/id' as datasets %}
{% get_set 'project/partner/id' as partners %}
{% get_set 'project/funder/id' as funders %}

{% for dataset in datasets %}
    {% get_set_value dataset 'project/dataset/preservation/repository_arrangements' as repository_arrangements %}
    <p><i>Dataset {{ dataset.value }}:</i>

    {% if repository_arrangements %}
        Number of arrangements: {{ repository_arrangements|length }}
    {% else %}
        Empty List
    {% endif %}
    </p>
    <p>{{ repository_arrangements }}</p>

{% endfor %}
jochenklar commented 2 years ago

Hi @Zack-83 , this is just a misunderstanding, if you use get_set_value you get only the first value of the collection, with get_set_values you get all. Example snippet:

{% get_set 'set/collection/text' as sets %}
{% for set in sets %}
    {% get_set_values set 'set/collection/text' as values %}

    {% if values|length > 0 %}
        Not empty!
    {% endif %}

    {% for value in values %}
    <p>{{ value.value }}</p>
    {% endfor %}
{% endfor %}
jochenklar commented 2 years ago

As you can see here: https://github.com/rdmorganiser/rdmo/blob/master/rdmo/views/templatetags/view_tags.py#L86 get_set_value is basically a short cut for get_set_values with index=0. You can also do things like {% get_set_value set 'set/collection/text' index=2 as values %} btw.

Zack-83 commented 2 years ago

Thanks @jochenklar ! That solves indeed part of the problem.

Could you also tell me why Python/Django sometimes renders an empty variable with "None" and some other times correctly skips it? What makes the difference?

Here is an example:

image

Rendering: https://rdmocat.aip.de/projects/13/views/8/

Source: https://rdmocat.aip.de/views/ --> horizon-europe

jochenklar commented 2 years ago

If the value is there, but empty it .value returns None: https://github.com/rdmorganiser/rdmo/blob/master/rdmo/projects/models/value.py#L158. If there is no value at all it is skipped. Not sure what the expected behaviour is here. You can use .value_and_unit to get an empty string if there is no Value: https://github.com/rdmorganiser/rdmo/blob/master/rdmo/projects/models/value.py#L164 Maybe we should rename this function.

Zack-83 commented 1 year ago

Thanks for the explanation.

If the value is there, but empty, then .value returns None: https://github.com/rdmorganiser/rdmo/blob/master/rdmo/projects/models/value.py#L158. If there is no value at all, it is skipped. Not sure what the expected behaviour is here.

I would expect the function to print nothing at all and thus I would remove the lines 157-158 & 164-165. But what does the rest of the community expect? How can we check it?

image

image

You can use .value_and_unit to get an empty string if there is no Value: https://github.com/rdmorganiser/rdmo/blob/master/rdmo/projects/models/value.py#L164 Maybe we should rename this function.

Nice to have. I guess it should not behave differently than .value, apart from the unit.

Zack-83 commented 1 year ago

If the value is there, but empty, then .value returns None: https://github.com/rdmorganiser/rdmo/blob/master/rdmo/projects/models/value.py#L158. If there is no value at all it is skipped.

Anyway, even {% if utility != '' %} or {% if utility != 'None' %} do not work.

Zack-83 commented 1 year ago

Hi @Zack-83 , this is just a misunderstanding, if you use get_set_value you get only the first value of the collection, with get_set_values you get all. Example snippet:

{% get_set 'set/collection/text' as sets %}
{% for set in sets %}
    {% get_set_values set 'set/collection/text' as values %}

    {% if values|length > 0 %}
      Not empty!
    {% endif %}

    {% for value in values %}
    <p>{{ value.value }}</p>
    {% endfor %}
{% endfor %}

Sorry to torture you. Possibly I still miss some Python/Django basics. So unfortunately I cannot fully understand your snippet, because (1) it reuses many times the same words and (2) as you correctly guessed, we have to do with two nested lists. :(

Might we try to rephrase it again corresponding to my real case?

Here is my attempt:

{% for dataset in datasets %}
    {% get_set_values dataset 'project/dataset/reuse_scenario' as utility %}
    {% if utility %}
        <p><i>Dataset {{ dataset.value }}:</i> The data might be useful to:</p>
        <ul>{% render_set_value_list dataset 'project/dataset/reuse_scenario' %}</ul>
    {% endif %}
{% endfor %}

{{ utility|length }} is equal to 1, even if the user clicked no option.

Zack-83 commented 1 year ago

Maybe @jwindeck or @MyPyDavid may help?

Might we try to rephrase it again corresponding to my real case?

jochenklar commented 1 year ago

Hey @Zack-83 , I think your problem can be solved with:

{% get_set 'project/dataset' as datasets %}

{% for dataset in datasets %}
    {% get_set_value dataset 'project/dataset/reuse_scenario' as value %}

    {% if not value.is_empty %}
        <p><i>Dataset {{ dataset.value }}:</i> The data might be useful to:</p>
        <ul>{% render_set_value_list dataset 'project/dataset/reuse_scenario' %}</ul>
    {% endif %}
{% endfor %}

get_set_value gives you the value with the set_index matching dataset and the collection_index=0 which is what you want here. If you have one value you can use .is_empty. The filters |is_empty etc are for lists, e.g. values|is_empty returns all empty values.

jochenklar commented 1 year ago

This is what the value looks like in the template: https://github.com/rdmorganiser/rdmo/blob/master/rdmo/projects/models/value.py#L104

This is what the tags and filters in view_tags do: https://github.com/rdmorganiser/rdmo/blob/master/rdmo/views/templatetags/view_tags.py

someone™️ should put that in a documentation sometime, somewhere ...

Zack-83 commented 1 year ago

Hey, thanks!

It is improving, but we're not done yet. :(

Same example, Question 21a:

{% if reuse_duration %} -->

image

{% if not reuse_duration.is_empty %} -->

image

The variable reuse_duration is empty for the third, fifth and sixth dataset, but apparently "empty in a different way". And I am not able to catch all of them with a single syntax.

image

Quite frustrating...

jochenklar commented 1 year ago

please post the view code ...

Zack-83 commented 1 year ago

please post the view code ...

    {% for dataset in datasets %}
        {% get_set_value dataset 'project/dataset/preservation/reuse_duration' as reuse_duration %}
        {% if reuse_duration %}
            <p><i>Dataset {{ dataset.value }}:</i> {{ reuse_duration.value }}.</p>
        {% endif %}
    {% endfor %}
jochenklar commented 1 year ago

Can you try {% if not reuse_duration.is_empty %} in the third line?

Zack-83 commented 1 year ago

I did (see example above, second half)

Zack-83 commented 1 year ago

I have an idea.

For a multiple-choice question, If a user clicks an option and subsequently unchecks it, the variable is saved as a list with an empty entry.

If the empty entry is the first one, than that ruins both all logical checks (|is_empty, |length) and the printing (yielding a list with initial empty bullet point). Even the function "View answers" fails :(

grafik

Unchecking an option should remove the corresponding list component, not just empty it.

Might that be the solution?

jochenklar commented 1 year ago

I just tried it on my instance with checkboxes, and the values are removed, when I deselect options. In your case a value with an empty string as text stays in the database?

In your case, does the question for reuse_duration use radio buttons or checkboxes?

MyPyDavid commented 1 year ago

we could fix some or most part of the bugs in the View in a meeting, here is a short overview:

Fixes for Q14 and Q38 which are also in the rdmocat.aip.de/projects/13/views/8/.

Q14

{% for dataset in datasets %}
    {% get_set_values dataset 'project/dataset/sharing/explanation' as sharing_explanation %}
    {% if sharing_explanation|is_not_empty  %}
        <p><i>Dataset {{ dataset.value }}:</i></p>
        <ul>
        {% for explanation in sharing_explanation|is_not_empty %}
        <li>{{ explanation.value }}</li>
        {% endfor %}
        </ul>
    {% endif %}
{% endfor %}

basically the same for Q38

{% for dataset in datasets %}
    {% get_set_values dataset 'project/dataset/data_security/security_measures' as security_measures %}
    {% if security_measures|is_not_empty %}
        <p><i>Dataset {{ dataset.value }}:</i></p>
        <ul>
        {% for measure in security_measures|is_not_empty %}
        <li>{{ measure.value }}</li>
        {% endfor %}
        </ul>
    {% endif %}         
{% endfor %}
jochenklar commented 1 year ago

{% if security_measures|is_not_empty %} makes no sense, see above. is_not_empty returns a list. Use


{% for security_measure in security_measures %}
    {% if security_measure.is_not_empty %}
    ...
    {% endfor %}
{% endfor %}
MyPyDavid commented 1 year ago

the whole if-block of {% if security_measures|is_not_empty %} should be skipped if there are no answers/non-empty-values given in the dataset that is why it's there... more explicit: {% if security_measures|is_not_empty != [] %}

Zack-83 commented 5 months ago

we could fix some or most part of the bugs in the View in a meeting

Would the two little edits in https://github.com/rdmorganiser/rdmo/compare/main...dev-UAGViews help solving the rest of the bugs?