Orcid api

[c-martinez]

Pull request details

Adds author information from ORCID API, when orcid field is filled. I'm sure it is not the most elegant solution, but it works and that is a good start. Suggestions for improvement are welcome.

As a contributor I confirm

• I read and followed the instructions in CONTRIBUTING.md
• The developer documentation is up to date with the changes introduced in this Pull Request
• Tests are passing
• All the workflows are passing

List of related issues or pull requests

Refs:

• Getting author information from orcid API #639
• First attempt to call orcid API #688

Describe the changes made in this pull request

Instructions to review the pull request

[abelsiqueira]

Hi @c-martinez, I appreciate the update on the PR. After rebasing and testing, it seems that everything is working fine. However, I have a few suggestions to improve the code further:

• It would be great if you could rebase into the newer main as there are conflicts that may have caused the tests not to run correctly.
• Running npm run dev throws various warnings and errors, mainly from the linter. Could you kindly take a look at those and fix them?
• To make it clear that users can import from ORCID, we should add a button, either on the top or on the side of the ORCID field. I'm not sure which would be better.
• We need a way to provide positive and negative feedback, maybe by using $q.notify as shown in Preview.vue.
• We should also add some tests to Cypress. Perhaps a separate test to verify that the workflow is functional would suffice.

Please let me know if you need any further input from me.

[c-martinez]

Hey @abelsiqueira, thanks for your feedback. I'm making a little checklist for myself, I'll do as much as I can, but I think there might be a couple of things where I will need help (javascript is not my strongest language):

• Rebase to newer main
• Run tests correctly
• Fix linter errors from npm run dev

I tried to fix most of these, but some I just had no idea how to fix. HELP!

• Add a button on top or side of ORCID field

Added some text on the top button, although I wasn't sure how to style the text (I wanted to add the text in a new line, with TIP on bold).

• Provide positive and negative feedback, maybe by using $q.notify as shown in Preview.vue.

Added positive feedback. Not sure where the negative feedback would need to be provided.

• We should also add some tests to Cypress

Not very sure how to do these last two (not familiar at all with Cypress).

[abelsiqueira]

Hi @c-martinez, thanks a lot for the update. I wasn't sure if I was supposed to already check this, but if I don't do it now, it might take another full week. You can @ me or click the "re-request review" when you want my input.

Hey @abelsiqueira, thanks for your feedback. I'm making a little checklist for myself, I'll do as much as I can, but I think there might be a couple of things where I will need help (javascript is not my strongest language):

Thanks for doing it. JS/TS is not my strongest either, especially on top of Vue, Quasar, etc. It's better that we take our time to do it right.

Rebase to newer main
Run tests correctly

Thanks for rebasing. The tests did not run because of a configuration file, so I need to ask you to rebase again (sorry). Should run now. By the way, we should try to make it work without disabling the linter. It might take longer to figure out how, but they help with best practices.

Fix linter errors from npm run dev

I tried to fix most of these, but some I just had no idea how to fix. HELP!

I found out how to fix one of the issues in a good way (I think). I made a suggestion on the code (the interface thing). The other one is related to props.$emit, which I am not sure how to fix either, but I left a link with some information about emit with the Composition API. Hopefully that moves things along.

Add a button on top or side of ORCID field

Added some text on the top button, although I wasn't sure how to style the text (I wanted to add the text in a new line, with TIP on bold).

I was thinking of an actual button close to the orcid field. Here is a suggestion of the UI:

With the button change, the emit could be like the other buttons that emit (I think), so that would be another problem solved.

Provide positive and negative feedback, maybe by using $q.notify as shown in Preview.vue.

Added positive feedback. Not sure where the negative feedback would need to be provided.

Nice positive feedback. The negative feedback is pretty much the same idea. When the button is pressed it the orcid is not successfully obtained (positive feedback), then something when wrong and notify with a negative color.
I see a two ways to handle these:

• Button can be clicked anytime. Needs to check for OrcidErrors as well as response.
• Button can only be clicked if there are no OrcidErrors. Needs to check for response.

I also realized that we might need a "waiting" feedback, i.e., the button is clicked and the response has not arrived yet. This is more complicated, so we can leave for future PRs.

We should also add some tests to Cypress

Not very sure how to do these last two (not familiar at all with Cypress).

We can do these after everything is done. Cypress simulates a user, "graphically" doing this. So we can write a few lines of code that test that the user can

• click on the field
• write the orcid
• click import button
• Verify that other fields are correct

It's not very complicated per se, but since there are requests now, things might get harder.

---

Thanks again for the updates, let me know if you need more clarification.

[abelsiqueira]

I don't know why this is needed, it doesn't look right.
You can change resp.config.props.$emit to this.$emit below, but that is the Options API, and we use the Composition API in this project. But I think that config.props = this is also not the right approach.
I think there is information here about how to do it, but I haven't really read it.
It might also be easier once we change to a button to import, instead of a watch. It can maybe work like other buttons that emit.

[abelsiqueira]

You should define $q so you don't need to reference it like this. There are other places that we use $q, it should be like those.

Suggested change

	resp.config.props.$q.notify({
	$q.notify({

[abelsiqueira]

I am rusty on the difference between Options API and Composition API, but I think this is Options API way, which we don't want. I think the Compositions API way to do it is to define a watchEffect or watch (same page) function inside the setup.
This also means that you don't need this, because the variables are all in scope.