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

Filetree_create survey question description #39

Closed
przemkalit opened this issue Nov 29, 2024 · 19 comments · Fixed by #46
Closed

Filetree_create survey question description #39

przemkalit opened this issue Nov 29, 2024 · 19 comments · Fixed by #46
Assignees
Labels
bug Something isn't working

Comments

@przemkalit
Copy link
Contributor

przemkalit commented Nov 29, 2024

Summary

Hi, I know, I've reported the #36 but at first I wanted to report the bug which is really annoying. Users cannot use quotes inside the question description.

Issue Type

  • Bug Report

Desired Behaviour

Surveys is properly exported.

Actual Behaviour

Surveys with quotes inside question description are not properly exported.

STEPS TO REPRODUCE

  1. Create JT/WF with survey question description, like below.
please copy-paste here the job id.  You'll find a "JOBS / ########" it in the top-left area of job's page: the #s are the job id.
  1. Export JT/WF

  2. Import JT/WF

@ivarmu
Copy link
Contributor

ivarmu commented Dec 11, 2024

@przemkalit I've completely changed the way that these fields were extracted and written down... I think I didn't forgive any case already solved in the previous PRs related with #36 and others... It would be great if you could test these changes and give feedback before merging the #46. Thanks!

@przemkalit
Copy link
Contributor Author

Okay, I found the first error. Since we have some workflows and job templates migrated from Tower, the multiplechoices field does not have max and min values set. As a result, the export is setting these to None, causing the import to fail.

@ivarmu
Copy link
Contributor

ivarmu commented Dec 11, 2024

Okay, I found the first error. Since we have some workflows and job templates migrated from Tower, the multiplechoices field does not have max and min values set. As a result, the export is setting these to None, causing the import to fail.

I'm looking into this...

@ivarmu
Copy link
Contributor

ivarmu commented Dec 11, 2024

Ok, right now... the empty fields shouldn't be exported, so no None should be a problem... but let's test again please @przemkalit :-)

@przemkalit
Copy link
Contributor Author

Sorry, it is no so simple, return value is equal Null, so the length is equal to 4 :-(.

@ivarmu
Copy link
Contributor

ivarmu commented Dec 11, 2024

Added the Null possibility :-D

@przemkalit
Copy link
Contributor Author

Ok I made mistake writing about Null because this is what API return but the YAML convert to None, but I did find that you were checking in the wrong place ;-). This is my creation:

{%   for survey_item in spec_item.spec if ((survey_item | string | length) > 0 and (survey_item | string) is not match('None')) %}
{%     set _var = namespace(first_survey_item = true) %}
{%     for survey_item_content in survey_item | dict2items %}
{%       if survey_item_content.key | regex_search('question_description|default') %}
{%         set current_content = survey_item_content.key + ': !unsafe "' + (survey_item_content.value | regex_replace("\n", "\\\\n") | regex_replace('"', '\\"')) + '"' %}
{%       elif survey_item_content.key is not match('choices') %}
{%           if (survey_item_content.value | string | length > 0) and (survey_item_content.value | string is not match('None')) %}
{%             set current_content = survey_item_content.key + ': ' + survey_item_content.value |string  %}
{%           endif %}
{%       endif %}
{%       if _var.first_survey_item and current_content is defined %}
{%         set _var.first_survey_item = false %}
        - {{ current_content }}
{%       else %}
{%         if current_content is defined %}
          {{ current_content | replace("$encrypted$", "\'\'") }}
{%         else %}
{%           if survey_item_content.key is match('choices') and survey_item_content.value[0] is defined %}
          {{ survey_item_content.key }}:
{%             for choice in survey_item_content.value %}
            - {{ choice | regex_replace('(^[^{]*){([{%])(.*)', '!unsafe "\\g<1>{\\g<2>\\g<3>"', multiline=True) }}
{%             endfor %}
{%           endif %}
{%         endif %}
{%       endif %}
{%     endfor %}
{%   endfor %}
      description: "{{ spec_item.description }}"
{% endfor %}

I left your check, but I've added check for survey_item_content.value, and my test success with minor hiccup, because I put only numbers into question name, but it was stupid example.

@ivarmu
Copy link
Contributor

ivarmu commented Dec 11, 2024

Let's check again. The idea is to check for empty values in the for, so no unneeded code is executed neither non useful output is generated.

@przemkalit
Copy link
Contributor Author

My main issue is that sometimes in API values of survey like min length or max length of answer for a question can be equal Null in API.

@ivarmu
Copy link
Contributor

ivarmu commented Dec 12, 2024

My main issue is that sometimes in API values of survey like min length or max length of answer for a question can be equal Null in API.

Ok, my idea, in that case, is that the for is discarding this item, either if it's value is 'Null' ('None' when in Ansible) or empty... Isn't it happening like described?

@przemkalit
Copy link
Contributor Author

przemkalit commented Dec 12, 2024

No, it is still returning None for min and max.

@ivarmu
Copy link
Contributor

ivarmu commented Dec 12, 2024

I think we have a version problem here... I'm testing against AAP 2.5 and all the survey items are returning the min and max with numerical values (0 and 1024 respectively). In older versions of AAP and Tower, it's right that it is returning Null for these fields, but this Git Repo is supposed to be compatible only for AAP >= 2.5, as it is using the gateway API to gather all the information.

I'm not sure how or where could we maintain the older versions of the collection for a reasonable amount of time, but I think that this shouldn't be the correct repo for this. Any thoughts about this @redhat-cop/automation-cop-tower-mgrs ?

@przemkalit
Copy link
Contributor Author

Ok, I think we will use our version of collection, until we migrate from 2.4 to 2.5, thus I cannot test your solution right now.

@ivarmu
Copy link
Contributor

ivarmu commented Dec 12, 2024

I'm missing something... I've different labs with AAP 2.2, 2.3 and 2.4, and all of them are returning these fields with the valid numeric values... if you test it against an AAP 2.4... you should get the correct values... Could you take a look at /api/v2/job_templates/<ID>/survey_spec/ ?

This is from an AAP 2.2:
image

@przemkalit
Copy link
Contributor Author

Yes, AAP returns proper values for all new survey question, but we migrated from Tower to AAP, which is why some survey questions have these issues.

@przemkalit
Copy link
Contributor Author

przemkalit commented Dec 12, 2024

I wanted some fail safe, that could managed this kind of issue if arise, but let's maybe ignore it, and I have it thanks to you effort :).

@ivarmu
Copy link
Contributor

ivarmu commented Dec 12, 2024

ok, so... it's not related to filetree_create, but with filetree_read role instead... This should be another issue maybe...

@ivarmu
Copy link
Contributor

ivarmu commented Dec 12, 2024

Returning to the current issue #39... Could you grab any other errors in your tests? If not, I'm asking to merge the PR #46.

@przemkalit
Copy link
Contributor Author

I've tested a really weird description and no issue, so you can merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants