oda-hub / frontend-tab-generator

0 stars 0 forks source link

oda_token is not parsed correctly from the front-end tab (the form truncates my string) #33

Closed ferrigno closed 7 months ago

ferrigno commented 8 months ago

I enetered my oda_token (a workaround until we have common roles across instances). However, the string is passed as "oda_token": ( "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJDYXJsby5GZXJyaWdub0B1bmlnZS5jaCIsIm5hbWUi" "OiJjZmVycmlnbm8iLCJleHAiOjE3MTM5NDc5OTg" ), according to the email I received with subject [ODA][submitted] lightcurve requested at 2024-02-27 15:59:24 job_id: 675d55bf

This is much smaller than the string of the actual token and my notebook fails.

Note that the first time I run the service, I received a message that I do not have enough privileges. Now, I get the actual error of the notebook and I can download the HTML of the notebook (Backend failed. DecodeError: Not enough segments)

dsavchenko commented 8 months ago

It appears that all the text fields in MMODA frontend have maxlength=128 set, which is not requested explicitly. Probably, drupal sets it while generating a form. @motame may clarify

The work-around would be to introduce another annotation, like e.g. oda:LongString class or oda:max_length property. The latter could be useful in other usecases, the former will require less effort to implement.

Based on this, generator may produce the textarea field

ferrigno commented 8 months ago

Note that his is a blocking issue for some use cases

ferrigno commented 8 months ago

Following a discussion, it looks that the issue is two fold:

volodymyrss commented 8 months ago

I made https://github.com/oda-hub/ontology/issues/24

* The user should be warned that a string is truncated when this is pasted in the box. This should be achieved by either a pop-up or by warning when it is passed to the back-end

To be clear, before it is sent to the dispatcher. This likely requires some JS UI by @motame .

motame commented 8 months ago

I am sorry but I am missing something here ! @ferrigno, where do you copy the oda token ? When the user clicks on "API token" : firefox_Ul2uIPsyj3

It's possible to copy it in the clipboard or receive it by email. I don't see what's wrong here !!

In which form do you copy it ?

motame commented 8 months ago

OK OK, I see it's about the tab generator ! @dsavchenko, how can I have a sample instrument to test it and propose a solution. Is it in Euclid form ?

ferrigno commented 8 months ago

It is related to the instance in Paris, where a tab is self- generated from a notebook. Nothing wrong with our mmoda web interface in Geneva, indeed.

dsavchenko commented 8 months ago

@dsavchenko, how can I have a sample instrument to test it and propose a solution.

It's in the staging.odahub.fr/mmoda, the "ISGRI-expert" instrument for example. It has a field "oda-token" which is intended to be filled with the token from the other (unige) instance (as a workaroung until we come up with some form of federation). The problem is that by default, a text field has the length restriction. Which is OK for most cases. If we really need the long text input, generator may generate textarea field instead of a simple text input. First question to you @motame is how to declare it in the .inc file?

And second thing on the frontend side, we need to somehow warn a user that his input is truncated in a simple text input (maybe only when it's actually truncated, e.g. user pasted too long input like in the case in hand).

motame commented 8 months ago

Cannot access it : firefox_rJ491N6WOh

dsavchenko commented 8 months ago

Cannot access it :

Let's check online on zoom / matrix chat if the issue persists

motame commented 8 months ago

@dsavchenko, how can I have a sample instrument to test it and propose a solution.

First question to you @motame is how to declare it in the .inc file? Here is a proposal :

$form['oda_token'] = array(
'#title' => t('ODA Token'),
'#type' => 'textarea',
'#description' => t('Description of the field'),
'#default_value' => 'Default value of the field if any',
'#required' => TRUE,
'#attributes' => array(
'maxlength' => '10000',
'class' => array(
'form-control'
)
),
'#parent_classes' => array(
'col-md-12'
),
'#prefix' => '<div class="row">',
'#suffix' => '</div>'
);

The field type should be "textarea" with an attribute maxlength which will be used in Javascript whether to accept or reject the paste.

And second thing on the frontend side, we need to somehow warn a user that his input is truncated in a simple text input (maybe only when it's actually truncated, e.g. user pasted too long input like in the case in hand). And here is the JS that warns the user ...

$("textarea").bind("paste", function(e) {
var pastedDataLength = e.originalEvent.clipboardData.getData('text').length;
if ($(this)[0].hasAttribute('maxlength') && !isNaN(parseInt($(this).attr('maxlength')))) {
var maxlength = parseInt($(this).attr('maxlength'));
if (pastedDataLength > maxlength) {
var parent_elt = $(this).parent().parent();
$('label', parent_elt).addClass('control-label');
parent_elt.addClass('has-error');
e.preventDefault();
console.log('Value to paste too large [max length : ' + maxlength + ']');
}
}
});

This code prevent the paste if the text to paste is longer than the allowed length (maxlength). Note that this code needs more work to show the error properly. It should be shown as the other errors of the form :

firefox_Jg4JqlRUKl

The used JS form validation library (bootstrapValidator) do not consider such a case : paste overflow !!!

dsavchenko commented 8 months ago

The field type should be "textarea" with an attribute maxlength which will be used in Javascript whether to accept or reject the paste.

Thank you! So this will work out-of-the-box, and we can just include it in the generator once it's added to the ontology and implemented in the dispatcher. Minor question, for the textarea, the number of rows/columns is set through #attributes array in the .inc file?

And the only change required in the frontend is to prevent pasting with proper error message. It may be anyway added independently. The only note is that it's needed for both textarea and simple text input (where maxlength is set by default to 128) (in your example code snippet it's only applied to the textarea)

motame commented 8 months ago

The field type should be "textarea" with an attribute maxlength which will be used in Javascript whether to accept or reject the paste.

Thank you! So this will work out-of-the-box, and we can just include it in the generator once it's added to the ontology and implemented in the dispatcher. Minor question, for the textarea, the number of rows/columns is set through #attributes array in the .inc file?

Not there but through #rows and there is also #cols

$form['description'] =array(
  '#type' => 'textarea', 
  '#title' => t('Description'), 
  '#default_value' => $edit['description'], 
  '#cols' => 60, 
  '#rows' => 5,
);

And the only change required in the frontend is to prevent pasting with proper error message. It may be anyway added independently. The only note is that it's needed for both textarea and simple text input (where maxlength is set by default to 128) (in your example code snippet it's only applied to the textarea)

Ah OK, text will be considered too.

motame commented 8 months ago

It's now implemented for the two types of inputs: text and textarea. Whenever the attribute maxlength, the paste is checked for the max length ... Here is an example ... chrome_QyBZfFVwoe

You can test it on https://dev.mtmco.net/mmoda/

volodymyrss commented 8 months ago

@ferrigno could you test it on https://dev.mtmco.net/mmoda/ ?

dsavchenko commented 8 months ago

@ferrigno could you test it on https://dev.mtmco.net/mmoda/ ?

That's only notification and paste-prevention. I think Carlo will want to check the other part, when the token is actually accepted. But this requires changes to ontology (and probably helper) + dispatcher (add the string length to the metadata) + tab generator. This is not done yet

However, the frontend part works, I checked it a bit. I think it could be merged.

ferrigno commented 8 months ago

It looks perfect, just be sure that the empty string (default) is in the form the field "ODA Token textarea" in production. And, indeed, I would like to test the actual implementation. Just let me know which ontology term I should use and I make the necessary change in my notebook. Do not forget to document it.

dsavchenko commented 7 months ago

Thanks @motame the frontend part is finished

I reopen the issue to track the tab generator part

dsavchenko commented 7 months ago

Currently we can just put large default length for all text inputs as a workaround

burnout87 commented 7 months ago

https://github.com/oda-hub/frontend-tab-generator/pull/34 to handle textarea generation from LongString parameters