Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { CreateNewVersion, PreprintStepperSelectors } from '@osf/features/prepri
import { IS_MEDIUM, pathJoin } from '@osf/shared/helpers';
import { DataciteTrackerComponent } from '@shared/components/datacite-tracker/datacite-tracker.component';
import { ReviewPermissions, UserPermissions } from '@shared/enums';
import { Identifier } from '@shared/models';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is an identifier? I don't see it in the code base.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

import { MetaTagsService } from '@shared/services';
import { ContributorsSelectors } from '@shared/stores';

Expand Down Expand Up @@ -290,12 +291,10 @@ export class PreprintDetailsComponent extends DataciteTrackerComponent implement
this.actions.resetState();
}

protected getDoi(): Observable<string | null> {
return this.preprint$.pipe(
filter((project) => project != null),
map((project) => project?.identifiers?.find((item) => item.category == 'doi')?.value ?? null)
);
protected override get trackable(): Observable<{ identifiers?: Identifier[] } | null> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this a collection of identifiers if you are only returning a preprint (singular)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the confusion, but I am not returning a preprint only. This method returns everything which has identifiers. In our case it it Preprints, Registrations and Projects.

These models have a collection of identifiers because there may be other identifiers other than DOI.

return this.preprint$;
}

fetchPreprintVersion(preprintVersionId: string) {
const currentUrl = this.router.url;
const newUrl = currentUrl.replace(/[^/]+$/, preprintVersionId);
Expand Down
10 changes: 4 additions & 6 deletions src/app/features/project/overview/project-overview.component.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We definitely need a unit test for this file. Even if you have to enable it in the jest.config file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done, thanks for the help

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { DialogService } from 'primeng/dynamicdialog';
import { Message } from 'primeng/message';
import { TagModule } from 'primeng/tag';

import { filter, map, Observable } from 'rxjs';
import { Observable } from 'rxjs';

import { CommonModule } from '@angular/common';
import {
Expand Down Expand Up @@ -35,6 +35,7 @@ import {
import { DataciteTrackerComponent } from '@shared/components/datacite-tracker/datacite-tracker.component';
import { Mode, ResourceType, UserPermissions } from '@shared/enums';
import { MapProjectOverview } from '@shared/mappers/resource-overview.mappers';
import { Identifier } from '@shared/models';
import { ToastService } from '@shared/services';
import {
ClearWiki,
Expand Down Expand Up @@ -191,11 +192,8 @@ export class ProjectOverviewComponent extends DataciteTrackerComponent implement
return null;
});

protected getDoi(): Observable<string | null> {
return this.currentProject$.pipe(
filter((project) => project != null),
map((project) => project?.identifiers?.find((item) => item.category == 'doi')?.value ?? null)
);
protected override get trackable(): Observable<{ identifiers?: Identifier[] } | null> {
return this.currentProject$;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this port to the other components? It's a nice touch the projects, I'm not certain without seeing the other components if there will be an observable of identifiers?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, It's already ported.
Preprints, Projects and Registrations have identifiers field

}

constructor() {
Expand Down
10 changes: 4 additions & 6 deletions src/app/features/registry/registry.component.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We definitely need a unit test for this file. Even if you have to enable it in the jest.config file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { select } from '@ngxs/store';

import { filter, map, Observable } from 'rxjs';
import { Observable } from 'rxjs';

import { DatePipe } from '@angular/common';
import { ChangeDetectionStrategy, Component, effect, HostBinding, inject } from '@angular/core';
Expand All @@ -10,6 +10,7 @@ import { RouterOutlet } from '@angular/router';
import { pathJoin } from '@osf/shared/helpers';
import { MetaTagsService } from '@osf/shared/services';
import { DataciteTrackerComponent } from '@shared/components/datacite-tracker/datacite-tracker.component';
import { Identifier } from '@shared/models';

import { RegistryOverviewSelectors } from './store/registry-overview';

Expand Down Expand Up @@ -42,11 +43,8 @@ export class RegistryComponent extends DataciteTrackerComponent {
this.setupDataciteViewTrackerEffect().subscribe();
}

protected getDoi(): Observable<string | null> {
return this.registry$.pipe(
filter((project) => project != null),
map((project) => project?.identifiers?.find((item) => item.category == 'doi')?.value ?? null)
);
protected override get trackable(): Observable<{ identifiers?: Identifier[] } | null> {
return this.registry$;
}

private setMetaTags(): void {
Expand Down

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We definitely need a unit test for this file. Even if you have to enable it in the jest.config file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is abstract class and it's logic is already covered with tests in child classes

Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
import { filter, Observable, switchMap, take } from 'rxjs';
import { filter, map, Observable, switchMap, take } from 'rxjs';

import { inject, Injectable } from '@angular/core';

import { Identifier } from '@shared/models';
import { DataciteService } from '@shared/services/datacite/datacite.service';

@Injectable()
export abstract class DataciteTrackerComponent {
private dataciteService = inject(DataciteService);

/**
* Abstract method to retrieve the DOI (Digital Object Identifier) of the resource.
* Abstract method to retrieve an observable of resource to be tracked.
* This method is generic enough to support all objects that have `identifiers` property.
* Must be implemented by subclasses.
*
* @returns An Observable that emits a string DOI or null if unavailable.
* @returns An Observable that emits an item which may contain DOI identifier or null .
*/
protected abstract getDoi(): Observable<string | null>;
protected abstract get trackable(): Observable<{ identifiers?: Identifier[] } | null>;

/**
* Sets up a one-time effect to log a "view" event to Datacite for the resource DOI.
Expand All @@ -24,7 +25,9 @@ export abstract class DataciteTrackerComponent {
* @returns An Observable that completes after logging the view.
*/
protected setupDataciteViewTrackerEffect(): Observable<void> {
return this.getDoi().pipe(
return this.trackable.pipe(
filter((item) => item != null),
map((item) => item?.identifiers?.find((identifier) => identifier.category == 'doi')?.value ?? null),
take(1),
filter((doi): doi is string => !!doi),
switchMap((doi) => this.dataciteService.logView(doi))
Expand Down
Loading