refactor(project landing page): use metadata endpoint to get data from backend (DSP-1199) #400

waychal · 2021-02-26T14:12:01Z

resolves DSP-1199

In this PR:

remove hardcoded metadata
remove interfaces written for project metadata
use data types (ProjectMetadata. Dataset, Person, Organisation, etc.) directly from js-lib
use Ajax request to get metadata from backend

mpro7 · 2021-03-01T08:14:25Z

src/app/project/board/attribution-tab-view/attribution-tab-view.component.ts


    // return the type of agent to use correct template to display it
-    getAgentType(agent: IPerson | IOrganisation) {
-        if (agent.hasOwnProperty('familyName')) {
+    setAgent (agent: Person | Organization | IId) {


Method return type is missing.

mpro7 · 2021-03-01T08:15:36Z

src/app/project/board/attribution-tab-view/attribution-tab-view.component.ts

+        let atype = this.getAgentType(agent);
+        if (atype) {
+            this.currentAgent = agent;
+            return atype;
+        }
+        this.currentAgent = this.subProperties[agent.id];
+        atype = this.getAgentType(this.currentAgent);
+        return atype;


why not if...else with one return outside?

When I used if..else before, linter was complaining about else condition and my tests were failing because of that. So I updated the code like this.

Same here, this answer.

mpro7 · 2021-03-01T08:17:05Z

src/app/project/board/attribution-tab-view/attribution-tab-view.component.ts

+    getAgentType (agent: Person | Organization | IId) {
+        if (agent instanceof Person) {
            return 'person';
        }
-        if (agent.hasOwnProperty('url')) {
+        if (agent instanceof Organization) {
            return 'organisation';
        }
+        return undefined;
    }


method return type is missing and if...else

linter was complaining same for else block here so I removed it.

I've applied below and on vscode, linter doesn't complain. Could you show me (screen) how it looks on your side?

setAgent (agent: Person | Organization | IId): string { let atype = this._datasetMetadataService.getContactType(agent); if (atype) { this.currentAgent = agent; } else { this.currentAgent = this.subProperties[agent.id]; atype = this._datasetMetadataService.getContactType(this.currentAgent); } return atype; }

Here it is: https://github.com/dasch-swiss/dsp-app/runs/1987482010?check_suite_focus=true.
It shows misplaced else error in multiple places.

It's like that because of wrong formatting (see your code in screen below). There shouldn't be ENTER between if and else blocks. See my example above (this code is from another comment - just miss clicked answering right comment, but the overall rule applies here too). Please adjust.

Thanks for pointing it out. Changes are done in d8b4993.

mpro7 · 2021-03-01T08:20:57Z

src/app/project/board/board.component.html

                    <!-- Functionality to be implemented -->
                    <a href *ngFor="let dformat of metadataDownloadFormats" class="download-metadata"> {{ dformat }} </a>


mpro7 · 2021-03-01T08:21:13Z

src/app/project/board/board.component.html

+                                [cdkCopyToClipboard]="selectedProject.id"
+                                (click)="copyToClipboard('Persistent identifier')"
+                                class="btn-copy-to-clipboard">
+                                <mat-icon [inline]="true" class="icon-arkurl">content_copy</mat-icon>


mpro7 · 2021-03-01T08:35:43Z

src/app/project/board/organisation-template/organisation-template.component.ts

+                <div class="property-label display-inline-block">
+                Email:
+                </div>


Indention or you can make it one-liner.

mpro7 · 2021-03-01T08:38:46Z

src/app/project/board/person-template/person-template.component.ts

+
+    ngOnInit() {
+        // check if members list is the list of [Organization] or [Iid]
+        let isOrganizationType: boolean = false;


If property is initiated with value, type annotation is redundant. It's not wrong, but not necessary for primitive types.

mpro7 · 2021-03-01T08:40:00Z

src/app/project/board/project-tab-view/project-tab-view.component.ts

+    SingleProject,
+    DataManagementPlan,
+    Person,
+    Organization,
+    IId,
+    Grant


alphabetise imports

mpro7 · 2021-03-01T08:42:08Z

src/app/project/board/project-tab-view/project-tab-view.component.ts

+
+    }
+
+    getDMP(currenDmps: DataManagementPlan | IId[]) {


return type is missing

mpro7 · 2021-03-01T08:43:33Z

src/app/project/board/project-tab-view/project-tab-view.component.ts

+    getFunderType(funder: Person | Organization | IId) {
+        if (funder instanceof Person) {
+            return 'person';
+        }
+        if (funder instanceof Organization) {
+            return 'organization';
+        }
+        return undefined;
+    }


like few comments above, this can be reused from service

waychal · 2021-03-02T09:14:43Z

Requested changes are done in 6bb29d8.

mpro7

Where possible, bring back correct form of if...else or if...else if blocks.

mpro7 · 2021-03-02T09:31:12Z

src/app/project/board/dataset-metadata.service.ts

+  getContactType(obj: Person | Organization | IId): string {
+    if (obj instanceof Person) {
+      return 'person';
+    }
+    if (obj instanceof Organization) {
+      return 'organization';
+    }
+    return undefined;
+  }


Use if..else if block here, have a look at this answer.

Thanks. Changes are done in d8b4993.

mpro7 · 2021-03-02T09:34:18Z

src/app/project/board/dataset-metadata.service.ts

+@Injectable({
+  providedIn: 'root'
+})
+export class DatasetMetadataService {


I would just name it MetadataService, or MetadataUtilService where all helpers method could be stored.

Also this repository has using 4 spaces indentions.

done in d8b4993.

…lable for selected project.

…adata structure

…ng to new metadata schema

…ntact page for new metadata schema Note: need to work on UI for person and organisation templates (mainly used in funder, grant, attribution and contact sections)

…adata is not available

…adata endpoint

…is ready

… identation;

…e service name;

mpro7

;)

mpro7 · 2021-03-02T11:08:37Z

src/app/project/board/dataset-metadata.service.ts

+        }
+        return undefined;


ending else is missing here

From my understanding if the case is default (and there is no further code to run in the function, we can add it without using else block. isn't it right?

mpro7 · 2021-03-02T11:23:01Z

src/app/project/board/dataset-metadata.service.ts

+        }
+        return undefined;


waychal added the refactor Refactor or redesign label Feb 26, 2021

waychal requested a review from mpro7 February 26, 2021 14:12

waychal self-assigned this Feb 26, 2021

mpro7 requested changes Mar 1, 2021

View reviewed changes

waychal force-pushed the wip/dsp-1199-use-metadata-endpoint-to-get-data branch from b688416 to 6bb29d8 Compare March 1, 2021 21:36

waychal requested a review from mpro7 March 2, 2021 09:14

mpro7 requested changes Mar 2, 2021

View reviewed changes

Snehal Kumbhar added 16 commits March 2, 2021 11:46

feat(project landing page): displayed message if metadata is not avai…

059249b

…lable for selected project.

feat(project landing page): removed copyToClipboard option for license

c659c11

refactor(project landing page): updated right side column for new met…

1f0a210

…adata structure

refactor(project landing page): update project and terms tabs accordi…

2f305f3

…ng to new metadata schema

refactor(project landing page): update dataset and attribution and co…

236fb93

…ntact page for new metadata schema Note: need to work on UI for person and organisation templates (mainly used in funder, grant, attribution and contact sections)

chore(project landing page): updated template for funder and grant

7baf0e2

chore(project landing page): updated style

2bd160f

chore(project landing page): cleanup, removed hardcoded metadata

3c2d630

refactor(project landing page): used snackbar to display error if met…

12315b9

…adata is not available

refactor(project landing page): updated error handler for project met…

aeff8f3

…adata endpoint

chore(project landing page): removed dataset-metadata interface

254e8f1

chore(project landing page): cleanup

21a5266

chore(project landing page): excluded some testcases until test data …

1db47bd

…is ready

chore(project landing page): solved lint errors

9d10dbf

chore(project landing page): use service to get contact type; correct…

dc0ef9c

… identation;

chore(project landing page): correct linter for if..else block; updat…

d8b4993

…e service name;

waychal force-pushed the wip/dsp-1199-use-metadata-endpoint-to-get-data branch from 6bb29d8 to d8b4993 Compare March 2, 2021 10:46

waychal requested a review from mpro7 March 2, 2021 11:00

mpro7 requested changes Mar 2, 2021

View reviewed changes

mpro7 approved these changes Mar 2, 2021

View reviewed changes

src/app/project/board/dataset-metadata.service.ts

Comment on lines +21 to +22

}

return undefined;

Copy link

Collaborator

mpro7 Mar 2, 2021

Choose a reason for hiding this comment

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

Correct.

waychal merged commit 5dde42f into main Mar 2, 2021

waychal deleted the wip/dsp-1199-use-metadata-endpoint-to-get-data branch March 2, 2021 12:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(project landing page): use metadata endpoint to get data from backend (DSP-1199) #400

refactor(project landing page): use metadata endpoint to get data from backend (DSP-1199) #400

waychal commented Feb 26, 2021

mpro7 Mar 1, 2021

mpro7 Mar 1, 2021

waychal Mar 1, 2021

mpro7 Mar 2, 2021

mpro7 Mar 1, 2021

waychal Mar 1, 2021

mpro7 Mar 2, 2021

waychal Mar 2, 2021 •

edited

mpro7 Mar 2, 2021 •

edited

waychal Mar 2, 2021

mpro7 Mar 1, 2021

mpro7 Mar 1, 2021

mpro7 Mar 1, 2021

mpro7 Mar 1, 2021

mpro7 Mar 1, 2021

mpro7 Mar 1, 2021

mpro7 Mar 1, 2021

waychal commented Mar 2, 2021

mpro7 left a comment

mpro7 Mar 2, 2021 •

edited

waychal Mar 2, 2021

mpro7 Mar 2, 2021

waychal Mar 2, 2021

mpro7 left a comment

mpro7 Mar 2, 2021

waychal Mar 2, 2021 •

edited

mpro7 Mar 2, 2021

mpro7 Mar 2, 2021

		<!-- Functionality to be implemented -->
		<a href *ngFor="let dformat of metadataDownloadFormats" class="download-metadata"> {{ dformat }} </a>

refactor(project landing page): use metadata endpoint to get data from backend (DSP-1199) #400

refactor(project landing page): use metadata endpoint to get data from backend (DSP-1199) #400

Conversation

waychal commented Feb 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waychal Mar 2, 2021 • edited

Choose a reason for hiding this comment

mpro7 Mar 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waychal commented Mar 2, 2021

mpro7 left a comment

Choose a reason for hiding this comment

mpro7 Mar 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpro7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waychal Mar 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waychal Mar 2, 2021 •

edited

mpro7 Mar 2, 2021 •

edited

mpro7 Mar 2, 2021 •

edited

waychal Mar 2, 2021 •

edited