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.

Sign up for GitHub

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

[ENG-4760] User secondary metadata #2031

Merged
merged 9 commits into from
Oct 20, 2023
Merged

[ENG-4760] User secondary metadata #2031

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
de2cb25
Add preprint relation to users
futa-ikeda Oct 16, 2023
b4f816c
Update result card for user info
futa-ikeda Oct 17, 2023
61b6624
Merge branch 'develop' into user-secondary-metadata
futa-ikeda Oct 17, 2023
4a77892
Group dt and dl a little closer
futa-ikeda Oct 17, 2023
fe8e44b
CR feedback; Refactor secondary metatadata components
futa-ikeda Oct 19, 2023
ed5f7bc
Merge branch 'develop' into user-secondary-metadata
futa-ikeda Oct 19, 2023
6880aa9
CR feedback; Add test selector
futa-ikeda Oct 20, 2023
18bc4dd
Better refactor
futa-ikeda Oct 20, 2023
9cbf67b
Remove leftover arg
futa-ikeda Oct 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/models/search-result.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,15 @@ export default class SearchResultModel extends Model {
return this.resourceMetadata['@id'];
}

// returns list of affilated institutions for users
// returns list of contributors for osf objects
// returns list of affiliated institutions for osf users
get affiliatedEntities() {
if (this.resourceType === 'user') {
// return something
if (this.resourceMetadata.affiliation) {
return this.resourceMetadata.affiliation.map((item: any) =>
({ name: item.name[0]['@value'], absoluteUrl: item['@id'] }));
aaxelb marked this conversation as resolved.
Show resolved Hide resolved
}
} else if (this.resourceMetadata.creator) {
return this.resourceMetadata.creator?.map((item: any) =>
({ name: item.name[0]['@value'], absoluteUrl: item['@id'] }));
Expand Down
4 changes: 4 additions & 0 deletions app/models/user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { buildValidations, validator } from 'ember-cp-validations';
import config from 'ember-osf-web/config/environment';
import { Link } from 'jsonapi-typescript';

import PreprintModel from 'ember-osf-web/models/preprint';
import SparseNodeModel from 'ember-osf-web/models/sparse-node';
import ContributorModel from './contributor';
import DraftRegistrationModel from './draft-registration';
Expand Down Expand Up @@ -114,6 +115,9 @@ export default class UserModel extends OsfModel.extend(Validations) {
@hasMany('draft-registration')
draftRegistrations!: AsyncHasMany<DraftRegistrationModel>;

@hasMany('preprints')
futa-ikeda marked this conversation as resolved.
Show resolved Hide resolved
preprints!: AsyncHasMany<PreprintModel>;

@hasMany('institution', { inverse: 'users' })
institutions!: AsyncHasMany<InstitutionModel>;

Expand Down
56 changes: 53 additions & 3 deletions lib/osf-components/addon/components/search-result-card/component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,81 @@
import { action } from '@ember/object';
import { inject as service } from '@ember/service';
import { waitFor } from '@ember/test-waiters';
import Store from '@ember-data/store';
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import SearchResultModel from 'ember-osf-web/models/search-result';
import { inject as service } from '@ember/service';
import Intl from 'ember-intl/services/intl';
import { dropTask } from 'ember-concurrency';
import { taskFor } from 'ember-concurrency-ts';

import config from 'ember-osf-web/config/environment';
import SearchResultModel from 'ember-osf-web/models/search-result';
import UserModel from 'ember-osf-web/models/user';

const osfUrl = config.OSF.url;

interface Args {
result: SearchResultModel;
}

export default class SearchResultCard extends Component<Args> {
@service intl!: Intl;
@service store!: Store;

@tracked isOpenSecondaryMetadata = false;
@tracked osfUser?: UserModel;

@action
toggleSecondaryMetadata() {
this.isOpenSecondaryMetadata = !this.isOpenSecondaryMetadata;
if (this.isUserResultCard && !this.osfUser) {
taskFor(this.getOsfUserModel).perform();
}
}

get cardTypeLabel() {
return this.intl.t(`osf-components.search-result-card.${this.args.result.resourceType}`);
}

// not sure if this is the best way, as there was a resourceType of "unknown" out in the wild
get isUserResultCard() {
return this.args.result.resourceType === 'user';
}

get secondaryMetadataComponent() {
const { resourceType } = this.args.result;

return `search-result-card/${resourceType.replace('_component', '')}-secondary-metadata`;
futa-ikeda marked this conversation as resolved.
Show resolved Hide resolved
}

@dropTask
@waitFor
async getOsfUserModel() {
const { result } = this.args;
if (result.resourceType === 'user') {
const { identifier } = result.resourceMetadata;
if (identifier) {
const guid = this.guidFromIdentifierList(identifier);
if (guid) {
const user = await this.store.findRecord('user', guid, {
adapterOptions: {
query: {
related_counts: 'nodes,registrations,preprints',
},
},
reload: true,
});
this.osfUser = user;
}
}
}
}

guidFromIdentifierList(ids: Array<{'@value': string}>) {
const osfId = ids.find(id => id['@value'].includes(osfUrl))?.['@value'];
if (osfId) {
// remove osfUrl from id and any leading/trailing slashes
return osfId.replace(osfUrl, '').replace(/^\/|\/$/g, '');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like a fragile way of fetching the user guid. Any thoughts/feedback on this would be appreciated!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would recommend startsWith (instead of includes) and slice (instead of replace), so the operation is anchored at the start of the string

for (const iri of ids.map(id => id['@value'])) {
    if (iri && iri.startsWith(osfUrl)) { 
        return iri.slice(osfUrl.length).replace(/^\/|\/$/g, '');
    } 
}

(does this happen other places? in backend code using IRIs as data, this sort of thing was common enough to warrant shared utility functions)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this could be done fairly simply with the URL object type by doing something like

for (const iri of irisArray ) {
   const url = new URL(iri);
   if (url.host === osfUrl) {
      return url.pathname.slice(1); // remove leading slash. Could also be slice(1,6)??
   }
}

Although this approach feels a bit too naive to make into a generalized util function

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want to compare hosts, yeah the built-in URL seems better than parsing it yourself (tho you'd also need to extract the host from osfUrl, since it's a full url) -- at the moment i'm more a fan of the primitive startswith/slice approach for iris within namespaces (one way to look at osf-id (osf-guid)), tho there's also the complication of osf-urls that are not osf-ids but are still used as identifiers...

for (const iri of ids.map(id => id['@value'])) {
    if (iri && iri.startsWith(osfUrl)) { 
        const pathSegments = iri.slice(osfUrl.length).split('/').filter(Boolean);
        if (pathSegments.length === 1) {
            return pathSegments[0];  // one path segment; looks like osf-id
        }
    } 
}

or using URL and comparing hosts:

const osfHost = new URL(osfUrl).host;
for (const iri of ids.map(id => new URL(id['@value']))) {
    if (iri.host === osfHost) { 
        const pathSegments = iri.pathname.split('/').filter(Boolean);
        if (pathSegments.length === 1) {
            return pathSegments[0];  // one path segment; looks like osf-id
        }
    } 
}

(the two would have different behavior if there were ever an osfUrl that had a non-empty path, which might be a nice future to aim for (supporting folks who want to deploy an instance at a subroute under their own domain) but is not the present we're in)

buuuut in any case, maybe the eventual helpful reusable thing would be a method on IndexCardModel to extract an osf-id from this.resourceIdentifier and/or this.resourceMetadata.identifier (and maybe another method to fetch the ember-data model instance for it) -- no pressure to add that now, tho

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not have caught the subtle difference in these two approaches. I'll opt for the first one that uses startsWith, as I think there may be some cases where the pathname approach may cause problems. Will add this guid-extration method and a fetchOsfModel, as I think that would be handy for possible future improvements

}
return null;
}
}
7 changes: 5 additions & 2 deletions lib/osf-components/addon/components/search-result-card/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,11 @@
padding-top: 10px;
}

dt,
dd {
dt {
margin: 15px 0 0 5px;
}

dd {
margin-left: 5px;
}
}
51 changes: 28 additions & 23 deletions lib/osf-components/addon/components/search-result-card/template.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,25 @@
<div local-class='type-label'>
{{this.cardTypeLabel}}
</div>
{{#if (not-eq @result.resourceType 'user')}}
<Button
{{on 'click' this.toggleSecondaryMetadata}}
aria-label={{if this.isOpenSecondaryMetadata
(t 'osf-components.search-result-card.hide_additional_metadata')
(t 'osf-components.search-result-card.show_additional_metadata')
}}
aria-controls={{secondaryMetadataPanelId}}
aria-expanded={{this.isOpenSecondaryMetadata}}
>
{{#if this.isOpenSecondaryMetadata}}
<FaIcon
@icon={{'angle-up'}}
/>
{{else}}
<FaIcon
@icon={{'angle-down'}}
/>
{{/if}}
</Button>
{{/if}}
<Button
{{on 'click' this.toggleSecondaryMetadata}}
aria-label={{if this.isOpenSecondaryMetadata
(t 'osf-components.search-result-card.hide_additional_metadata')
(t 'osf-components.search-result-card.show_additional_metadata')
}}
aria-controls={{secondaryMetadataPanelId}}
aria-expanded={{this.isOpenSecondaryMetadata}}
>
{{#if this.isOpenSecondaryMetadata}}
<FaIcon
@icon={{'angle-up'}}
/>
{{else}}
<FaIcon
@icon={{'angle-down'}}
/>
{{/if}}
</Button>
</div>
<h4>
<a data-test-search-result-card-title href={{@result.absoluteUrl}} target='_blank' rel='noopener noreferrer'> {{@result.displayTitle}} </a>
Expand Down Expand Up @@ -100,8 +98,15 @@
<CpPanel local-class='cp-panel' id={{secondaryMetadataPanelId}} @open={{this.isOpenSecondaryMetadata}} as |panel|>
<panel.body>
<hr>
{{component this.secondaryMetadataComponent result=@result}}
{{#if this.isUserResultCard}}
<SearchResultCard::UserSecondaryMetadata
@user={{this.osfUser}}
@fetchTask={{this.getOsfUserModel}}
/>
{{else}}
{{component this.secondaryMetadataComponent result=@result}}
{{/if}}
futa-ikeda marked this conversation as resolved.
Show resolved Hide resolved
</panel.body>
</CpPanel>
{{/let}}
</div>
</div>
24 changes: 24 additions & 0 deletions lib/osf-components/addon/components/search-result-card/user-secondary-metadata/template.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{{#if @fetchTask.isRunning}}
<LoadingIndicator @dark={{true}}/>
{{else if @fetchTask.isError}}
{{t 'osf-components.search-result-card.fetch_user_error'}}
{{else if (not @user)}}
{{t 'osf-components.search-result-card.no_user'}}
{{else}}
<dl>
{{#if @user.employment.length}}
<dt>{{t 'osf-components.search-result-card.employment'}}</dt>
<dd>{{@user.employment.[0].institution}}</dd>
{{/if}}
{{#if @user.education.length}}
<dt>{{t 'osf-components.search-result-card.education'}}</dt>
<dd>{{@user.education.[0].institution}}</dd>
{{/if}}
<dt>{{t 'osf-components.search-result-card.public_projects'}}</dt>
<dd>{{@user.relatedCounts.nodes}}</dd>
<dt>{{t 'osf-components.search-result-card.public_registrations'}}</dt>
<dd>{{@user.relatedCounts.registrations}}</dd>
<dt>{{t 'osf-components.search-result-card.public_preprints'}}</dt>
<dd>{{@user.relatedCounts.preprints}}</dd>
</dl>
{{/if}}
1 change: 1 addition & 0 deletions lib/osf-components/app/components/search-result-card/user-secondary-metadata/template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from 'osf-components/components/search-result-card/user-secondary-metadata/template';
8 changes: 8 additions & 0 deletions mirage/serializers/user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ export default class UserSerializer extends ApplicationSerializer<User> {
},
},
},
preprints: {
links: {
related: {
href: `${apiUrl}/v2/users/${model.id}/preprints/`,
meta: this.buildRelatedLinkMeta(model, 'preprints'),
},
},
},
};
if (model.defaultRegion) {
serializedRelationships.defaultRegion = {
Expand Down
7 changes: 7 additions & 0 deletions translations/en-us.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1896,6 +1896,13 @@ osf-components:
withdrawn: Withdrawn
unknown: Unknown
remaining_count: '{count} more'
fetch_user_error: 'Unable to fetch user information from OSF'
no_user: 'No user'
employment: Employment
education: Education
public_projects: Public projects
public_registrations: Public registrations
public_preprints: Public preprints
resources-list:
add_instructions: 'Link a DOI from a repository to your registration by clicking the green “+” button.'
add_instructions_adhere: 'Contributors affirmed to adhere to the criteria for each badge.'
Expand Down