-
Notifications
You must be signed in to change notification settings - Fork 32
feat: templated external links #2194
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
base: master
Are you sure you want to change the base?
Changes from all commits
7cd9abe
a5ac1cc
ae07731
c403b88
c0fe37d
0adb721
6683b8a
a7fba61
47838fe
a289928
dfb0a78
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| [ | ||
| { | ||
| "title": "Franzviewer II", | ||
| "url_template": "https://franz.site.com/franzviewer?id=${dataset.pid}", | ||
| "description_template": "View ${dataset.numberOfFiles} files in Franz' own personal viewer", | ||
| "filter": "(dataset.type == 'derived') && dataset.owner.includes('Franz')" | ||
| }, | ||
| { | ||
| "title": "High Beam-Energy View", | ||
| "url_template": "https://beamviewer.beamline.net/highenergy?id=${dataset.pid}", | ||
| "description_template": "The high-energy beamviewer (value ${dataset.scientificMetadata?.beamEnergy?.value}) at beamCo", | ||
| "filter": "(dataset.scientificMetadata?.beamEnergy?.value > 20)" | ||
| } | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,7 @@ import { | |
| IDatasetRelation, | ||
| IDatasetScopes, | ||
| } from "./interfaces/dataset-filters.interface"; | ||
| import { ExternalLinkClass } from "./schemas/externallink.class"; | ||
| import { DatasetClass, DatasetDocument } from "./schemas/dataset.schema"; | ||
| import { | ||
| DATASET_LOOKUP_FIELDS, | ||
|
|
@@ -485,6 +486,56 @@ export class DatasetsService { | |
| throw new NotFoundException(error); | ||
| } | ||
| } | ||
|
|
||
| async findExternalLinksById(id: string): Promise<ExternalLinkClass[]> { | ||
| const thisDataSet = await this.findOneComplete({ | ||
| where: { pid: id }, | ||
| include: [DatasetLookupKeysEnum.all], | ||
| }); | ||
|
|
||
| if (!thisDataSet) { | ||
| // no luck. we need to create a new dataset | ||
| throw new NotFoundException(`Dataset #${id} not found`); | ||
| } | ||
|
|
||
| interface ExternalLinkTemplateConfig { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be nice to have interfaces/types in separate file, but that it just my subjective view.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general I agree. Right now this type is only used once, and in this spot. But I think we should be using types like this when we're parsing/validating all the JSON that this config data comes from. That may be a task best done en-masse though, as a separate change... |
||
| title: string; | ||
| url_template: string; | ||
| description_template: string; | ||
| filter: string; | ||
| } | ||
|
|
||
| const templates: ExternalLinkTemplateConfig[] | undefined = | ||
| this.configService.get("datasetExternalLinkTemplates"); | ||
| if (!templates) { | ||
| return []; | ||
| } | ||
|
|
||
| return templates | ||
| .filter((template) => { | ||
| const filterFn = new Function( | ||
| "dataset", | ||
| `return (${template.filter});`, | ||
| ); | ||
| return filterFn(thisDataSet); | ||
| }) | ||
| .map((template) => { | ||
| const urlFn = new Function( | ||
| "dataset", | ||
| `return (\`${template.url_template}\`);`, | ||
| ); | ||
| const descriptionFn = new Function( | ||
| "dataset", | ||
| `return (\`${template.description_template}\`);`, | ||
| ); | ||
| return { | ||
| url: urlFn(thisDataSet), | ||
| title: template.title, | ||
| description: descriptionFn(thisDataSet), | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| // Get metadata keys | ||
| async metadataKeys( | ||
| filters: IFilters<DatasetDocument, IDatasetFields>, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { ApiProperty } from "@nestjs/swagger"; | ||
| import { IsString } from "class-validator"; | ||
|
|
||
| // This class defines the externalLinks field in a dataset. | ||
| // That field is not represented in the Mongoose data store, | ||
| // so there is no equivalent schema representation for it. | ||
|
|
||
| export class ExternalLinkClass { | ||
| @ApiProperty({ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could consider having these api properties automatically generated as we for example did in the samples module (if I remember correctly) let me know if you want more information
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean by using |
||
| type: String, | ||
| required: true, | ||
| description: "URL of the external link.", | ||
| }) | ||
| @IsString() | ||
| readonly url: string; | ||
|
|
||
| @ApiProperty({ | ||
| type: String, | ||
| required: true, | ||
| description: "Text to display representing the external link.", | ||
| }) | ||
| @IsString() | ||
| readonly title: string; | ||
|
|
||
| @ApiProperty({ | ||
| type: String, | ||
| required: false, | ||
| description: "Description of the link destination.", | ||
| }) | ||
| @IsString() | ||
| readonly description?: string; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add new endpoints to the old v3 controller?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what the right move is, honestly.
I'd be happy to add them but it would of course be a change to the v3 API (though only an additive one). Are we still changing that API or is it in a static state now that we have v4?