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

implements URO-211 part B - link creation #221

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

implements URO-211 part B - link creation #221

wants to merge 16 commits into from

Conversation

eapearson
Copy link
Contributor

@eapearson eapearson commented Jun 26, 2024

[ DRAFT ] - still need to create the PR description, observe the GHA outcome, and respond to code quality alerts. Then I'll request a review. Not sure I'll have time to respond to all requests for change before the EOD Friday, but we can try.
The purpose of this PR is to introduce the functionality and user interface to support the creation of an ORCID Link. It builds upon previous work to establish the basic UI components and testing support.

It does not include the following functionality:

  • create link from other location (i.e. user profile)
  • manage own link
  • orcidlink manager manages orcidlink service

The latter two will be implemented in a follow-up PR (or at least branch).

The main area of concern in this PR is probably the approach to interacting with the orcidlink service during linking. Because the linking process is fleeting, and doesn’t really affect application state, a non-RTK client is used in some places. This is also because I didn’t have time to understand RTK well enough to implement this style of query. Anyway, the code for this was extracted from kbase-ui, modified to fit, and (new) tests written.

Another area of note is the usage of “compound components”, in which a stack of related components implements the navigation entry points. It is similar to MVC, in that there is a controller, and there is a view. The controller is responsible for fetching data and providing action functions to the view, and the view is primarily responsible for the ui (although the controller may display loading and error messages). In some cases there is also an “entrypoint” component which is responsible for ensuring that route parameters are populated. The latter is required by various factors, such as the optional nature of route params and search params and the decoupling of app state and component flow (e.g. getting auth state even when behind the Authed component).

I imagine that some of these implementation decisions could be solved by other means.

Not an effort to fully correct the typing for JSON-RPC 2.0, just this case which interfered with using it.
Added because existing loader does not allow for additional content to entertain the user
used to inform the user how long their linking session has left before expiration
ensures consistent formatting of a user's ORCID Id, compliant with ORCID branding requirements
A common error is simply one thrown w/in the ui, with some extra context to help the user.
either moved to local components if used once, or removed, if not necessary, or moved to common/style.module.scss
it was only used in this component
ported over from kbase-ui and modified to fit;  can be replaced by RTK later
part 1, mostly a placeholder to receive the redirect; second installment will add the service call to fetch the error definition.
changes made around the linking process and improvements.
sorry for the import reordering diff, it had to be done at some point (!) and otherwise the only functional changes are in the orcidlink routes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant