Add tests to formdata_to_model regarding citeable

[jmartinm]

A quick fix was introduced in #1979 to adapt to latest inspire-schemas.

Proper tests need to be added to make sure that the form is adding the citable flag properly. Also the flag refereed needs to be checked.

[rikirenz]

Citeable:
If we submitted from literature suggest form an article we consider it citeable if it has all the following fields set properly:

• year
• journal_title
• journal_volume

And at least one between:

• page_start
• artid

Here there is the code that is doing this consideration: https://github.com/inspirehep/inspire-schemas/blob/master/inspire_schemas/builders.py#L272&L300.

The tests should be easy but I think should be done in inspire_schema rather than in inspire_next
@jmartinm what do you think?

Refereed:
Now we do not populate the field refereed. Should be populated? if the answer is yes, let me know how we should populate that field.

@michamos let us know what we should do 🗣️ with the refereed field

[jmartinm]

Now we do not populate the field refereed. Should be populated? if the answer is yes, let me know how we should populate that field.

The equivalent before the big inspire-schemas merge was to add collections:Published (see 4965064#diff-7855ab3082d8e025a3787d156d10988dL218). The equivalent now would be refereed:True @michamos can confirm.

The tests should be easy but I think should be done in inspire_schema rather than in inspire_next
@jmartinm what do you think?

I personally liked the unit tests that were in test_literaturesuggest_tasks.py (like 4965064#diff-7855ab3082d8e025a3787d156d10988dL197) and did not realise they got removed. You can discuss with @jacquerie

[jacquerie]

I personally liked the unit tests that were in test_literaturesuggest_tasks.py (like 4965064#diff-7855ab3082d8e025a3787d156d10988dL197) and did not realise they got removed.

I took the decision to remove them in the interest of speed in #1918. That design was and still is better than the current one in tests/integration/test_formdata_to_model.py.

[rikirenz]

I will do it in the way that @jacquerie suggest that is the old one that we were using in the test_literaturesuggest_tasks.py. Anyway at that time we had logic inside the formdata_to_model that was supposed to be tested but now we have moved that logic in the builder. For this reason I think is not 100% useful write those tests for formdata_to_model.

But please let me know why you ( @jmartinm @jacquerie ) like them or why you prefer those rather than the actual ones. Because probably you are right but I am not able to see your point.

[jacquerie]

Anyway at that time we had logic inside the formdata_to_model that was supposed to be tested but now we have moved that logic in the builder.

That's the key part you are mistaken about: the builder is (almost) free of logic, and that logic is still in formdata_to_model. For example:

inspire-next/inspirehep/modules/literaturesuggest/tasks.py

Lines 204 to 209 in 6aaa968

	if not _is_arxiv_url(form_fields.get('url', '')):
	builder.add_url(url=form_fields.get('url'))
	obj.extra_data['submission_pdf'] = form_fields.get('url')
	if not _is_arxiv_url(form_fields.get('url', '')):
	builder.add_url(url=form_fields.get('additional_url'))

this is logic, and it can be unit tested.

[jmartinm]

Partially done in inspirehep/inspire-schemas#118

What about refereed? Is formdata_to_model populating it? Does it need to?

[rikirenz]

@michamos told me that for know is not so easy fill that field. I do not know if @kaplun has a strategy to do it?

[kaplun]

Yeah that property really depends on the specific journal to which the record is associated, and even within a given journal it's not necessary all the volume/issues to be refereed (e.g. some journal publishes both conference proceedings and articles).
So we need a more deep discussion about this, but I believe that for the time being we can move on without this field being populated and then fill-it in asynchronously (e.g. in the workflow or with some deamon...)

[rikirenz]

Do you think we should open an issue for this?

[jacquerie]

What about refereed? Is formdata_to_model populating it?

No, so I think that this issue can be closed, since we already have one about implementing refereed: inspirehep/inspire-schemas#117 (we don't even know the requirements yet).