16 sql export fix variable management imp

[legalsylvain]

Finish OCA#718

CC : @bguillot, @florian-dacosta

• Add required, so the display mentions that the fields are mandatory :
  

• add a check before execution, before the required params is not handled correctly for Properties

• remove obsolete nodestroy params

[florian-dacosta]

@legalsylvain Thanks a lot for your PR!!

I agree this is an issue to have an error in case of empty value in string. I had not seen any obvious solution for this, with this property fields.
From what I see, making it require make all properties in blue like it is mandatory, but it does not block anything, that is why you add a raise during the validation of the wizard.

I think it is good enough indeed. But, we can't make the query_properties required on the python definition. Indeed, all the queries that work without any parameter would be blocked. I just tested and I have the error :

The operation cannot be completed:
- Create/update: a mandatory field is not set.
- Delete: another model requires the record being deleted. If possible, archive it instead.

Model: Allow the user to save the file with sql request's data (sql.file.wizard)
Field: Properties (query_properties)

But, I tried to add a required="1" in the view on query_properties, and it seems to work fine.
I mean, the field is blue and seems mandatory, but no problem to run queries without parameters.
The field are still not mandatory (it just seems to be because it is blue..) so the raise during wizard validation is still usefull.

So, just change the require from the python field definition to the view and it is ok for me!
(If you want I do a PR on your PR)
Thanks again for this improvement !

[legalsylvain]

I think it is good enough indeed. But, we can't make the query_properties required on the python definition. Indeed, all the queries that work without any parameter would be blocked. I just tested and I have the error :

Oh, I didn't saw that corner case. Thanks !

So, just change the require from the python field definition to the view and it is ok for me!

done. Let me know !

[florian-dacosta]

LGTM, I have tested and works well