removed dataset enrichment from dcat plugin #31

sansth1010 · 2023-02-01T16:02:07Z

Please ignore the failed test cases for now. It will be resolved before the merge to beta branch.

src/dcat-us/dataset-formatter.ts

rgwozdz · 2023-02-01T20:44:06Z

src/index.ts

-
-      if (!dcatConfig) {
-        dcatConfig = _.get(siteModel, 'data.feeds.dcatUS11');
+      const { res: { locals: { feedTemplate, feedTemplateTransforms, nonEditableTemplateFields } } }: {


You should probably allow for the feedTemplate to arrive on the req.query as well. req.query.feedTemplate used by default, if not, check in locals.

Where do feedTemplateTransforms and nonEditableTemplateFields come from?

@rgwozdz feedTemplateTransforms and nonEditableTemplateFields comes from Koop instance and is part of feed templates.

can you link to where we assign to locals?

You should probably allow for the feedTemplate to arrive on the req.query as well. req.query.feedTemplate used by default, if not, check in locals.

Do we allow feed template through query in output plugin as well? The logic of converting template passed via query to feedTemplate object already exists in our koop instance (hub-feeds-api). Regular users would just create feedTemplate object pass it to locals in koop instance. We can implement parsing template via query in output plugin itself but should remove the query template parsing logic from hub feeds api.

can you link to where we assign to locals?

It can be found here. Recent PR on hub feeds api. https://devtopia.esri.com/dc/hub-feeds-api/pull/128

rgwozdz · 2023-02-01T21:14:54Z

src/index.ts

-      // Request a single dataset if id is provided, else default to site's catalog
-      const id = String(req.query.id || '');
-      req.res.locals.searchRequest = this.getDatasetSearchRequest(id, apiTerms) || this.getCatalogSearchRequest(req, _.get(siteModel, 'data.catalog'), apiTerms);
+      const { stream: dcatStream } = getDataStreamDcatUs11(feedTemplate, feedTemplateTransforms, nonEditableTemplateFields);


Seems like this will fail if the user's Koop instance doesn't add all these things to locals. Are these parameters things that could conceivable arrive on the req.query or req.body? Also are there default we can use in the event the request doesn't have all of these? If not, it would seem that we would want to a 400 error.

Are these parameters things that could conceivable arrive on the req.query or req.body?

Currently, we expect users using the plugin to have a feed template object in their koop instance but we can allow template to be passed via req.query or req.body but should remove existing template parsing via query in hub feeds api as it would be redundant.

Also are there default we can use in the event the request doesn't have all of these? I

We could have a default one but dcat us feed has properties like title, landingPage whose values are dynamic. Feed template is essential to map these keys to appropriate values from datasource. There is no common data source as providers could be different. so, common mapping is not possible.

it would seem that we would want to a 400 error.

Currently responds if 400 error if feedTemplate not set.

rgwozdz · 2023-02-03T18:22:57Z

src/index.ts

+      } = req;
+
+      if (!feedTemplate) {
+        throw new RemoteServerError('DCAT-US 1.1 feed template is not provided.', null, 400);


You don't want a "RemoteServerError" as it indicates the error originates from some other upstream server. You'd use this to distinguish an error originating from the hub-search service as opposed to the Koop feeds service.

Created Custom error for errors originating at DCAT plugin.

rgwozdz · 2023-02-03T18:24:12Z

src/dcat-us/dataset-formatter.ts

+      theme: dcatDataset.spatial && ['geospatial']
+    }, null, '\t'), 2);
+  } catch (err) {
+    throw new RemoteServerError(err?.message || 'Error parsing feed template', null, 400);


You don't want a "RemoteServerError" as it indicates the error originates from some other upstream server. You'd use this to distinguish an error originating from the hub-search service as opposed to the Koop feeds service.

Created Custom error for errors originating at DCAT plugin.

rgwozdz · 2023-02-03T18:30:12Z

src/dcat-us/dataset-formatter.ts

-    hubDataset.license = null;
-  }
-
+export function formatDcatDataset(hubDataset: any, feedTemplate: DcatDatasetTemplate, feedTemplateTransforms: TransformsList) {


This is the output plugin so it shouldn't have any knowledge that it is working with hubDatasets, so you should change the name of the param to something generic. Let's also change the name of this function to compileDcatFeedEntry.

Agreed! renamed hubDataset to feedData and renamed formatDcatDataset to compileDcatFeedEntry

rgwozdz · 2023-02-03T18:30:53Z

src/dcat-us/index.test.ts

-) {
-  const siteModel = { item: { url: hostname } } as unknown as IModel;
+async function generateDcatFeed(dataset, template, templateTransforms) {
+  // const siteModel = { item: { url: hostname } } as unknown as IModel;


Remove comment I think.

Yes. Comment has been removed.

rgwozdz · 2023-02-03T18:32:11Z

src/dcat-us/dataset-formatter.ts


-  return indent(JSON.stringify(dcatDataset, null, '\t'), 2);
+function removeUninterpolatedDistributions(distributions: any[]) {
+  return distributions.filter((distro) => !(typeof distro === 'string' && distro.match(/{{.+}}/)?.length));
 }

 // HUBJS CANDIDATE
 function indent(str: string, nTabs = 1) {


Is indenting required for feeds to work? It will increase the payload size.

I agree but this indenting might have been one of the requirements for displaying feed. @thomas-hervey can let us know. Otherwise, we can remove the indentation entirely.

rgwozdz · 2023-02-06T14:08:32Z

src/index.ts

-import { fetchSite, getHubApiUrl, getPortalApiUrl, hubApiRequest, IHubRequestOptions, RemoteServerError } from '@esri/hub-common';
-import { IContentSearchRequest, SortDirection } from '@esri/hub-search';
-
+import { RemoteServerError } from '@esri/hub-common';


I think this import is no longer require?

Unused import has been removed.

rgwozdz

Looks good, just the one import to remove.

Is there any coverage tooling on this repo? Curious where we are in terms of test coverage.

sansth1010 · 2023-02-07T00:58:11Z

@rgwozdz : Test coverage as follows:

I do not think there is a coverage tooling on this repo yet but we can set it up.

removed dataset enrichment from dcat plugin

8b383d9

sansth1010 requested review from rgwozdz, sonofflynn89 and drspacemanphd February 1, 2023 16:02

sansth1010 changed the title ~~removed dataset enrichment from dcat plugin~~ [DNM] - removed dataset enrichment from dcat plugin Feb 1, 2023

sansth1010 changed the base branch from main to beta February 1, 2023 17:03

sansth1010 added the draft label Feb 1, 2023

rgwozdz reviewed Feb 1, 2023

View reviewed changes

src/dcat-us/dataset-formatter.ts Outdated Show resolved Hide resolved

rgwozdz reviewed Feb 1, 2023

View reviewed changes

sansth1010 added 2 commits February 2, 2023 04:07

remove noneditable formating from output plugin

176833a

fix tests

cf6599e

sansth1010 requested a review from rgwozdz February 3, 2023 06:50

sansth1010 removed the draft label Feb 3, 2023

sansth1010 changed the title ~~[DNM] - removed dataset enrichment from dcat plugin~~ removed dataset enrichment from dcat plugin Feb 3, 2023

rgwozdz requested changes Feb 3, 2023

View reviewed changes

sansth1010 added 2 commits February 3, 2023 17:53

renaming and custom error for dcat specific errors

00ec43c

remove dead comments

b2bba39

sansth1010 requested a review from rgwozdz February 4, 2023 04:52

add tests and custom error

4a17736

rgwozdz reviewed Feb 6, 2023

View reviewed changes

rgwozdz approved these changes Feb 6, 2023

View reviewed changes

remove unused import and add test cases

9d4f7fc

sansth1010 merged commit 7ee42b3 into beta Feb 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

removed dataset enrichment from dcat plugin #31

removed dataset enrichment from dcat plugin #31

sansth1010 commented Feb 1, 2023

rgwozdz Feb 1, 2023

rgwozdz Feb 1, 2023

sansth1010 Feb 1, 2023

rgwozdz Feb 1, 2023

sansth1010 Feb 2, 2023

rgwozdz Feb 1, 2023

sansth1010 Feb 2, 2023

rgwozdz Feb 3, 2023

sansth1010 Feb 4, 2023

rgwozdz Feb 3, 2023

sansth1010 Feb 4, 2023

rgwozdz Feb 3, 2023

sansth1010 Feb 4, 2023

rgwozdz Feb 3, 2023

sansth1010 Feb 4, 2023

rgwozdz Feb 3, 2023

sansth1010 Feb 4, 2023

rgwozdz Feb 6, 2023

sansth1010 Feb 7, 2023

rgwozdz left a comment

sansth1010 commented Feb 7, 2023

removed dataset enrichment from dcat plugin #31

removed dataset enrichment from dcat plugin #31

Conversation

sansth1010 commented Feb 1, 2023

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

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

Choose a reason for hiding this comment

rgwozdz left a comment

Choose a reason for hiding this comment

sansth1010 commented Feb 7, 2023