culturecreates / footlight-aggregator

A tool to inject entities from Artsdata to footlight
0 stars 0 forks source link

Aggregator support for type array - Blue Jean Bleu #19

Closed saumier closed 9 months ago

saumier commented 1 year ago

Artsdata returned the following data for Blue Jean Bleu organization

http://api.artsdata.ca/query?adid=K10-580&format=json&frame=ranked_org_person_footlight&sparql=ranked_org_person_footlight

{
  "status": "success",
  "meta": {
    "sparql": "ranked_org_person_footlight",
    "frame": "ranked_org_person_footlight",
    "limit": "10",
    "offset": "0",
    "adid": "K10-580",
    "format": "json"
  },
  "data": [
    {
      "uri": "http://kg.artsdata.ca/resource/K10-580",
      "type": [
        "Organization",
        "MusicGroup"
      ],
      "name": {
        "fr": "Bleu Jeans Bleu",
        "en": "Bleu Jeans Bleu"
      },
      "sameAs": [
        {
          "uri": "http://kg.artsdata.ca/resource/K10-580"
        }
      ],
      "url": {
        "uri": "http://www.bleujeansbleu.com/"
      },
      "contactPoint": null
    }
  ]
}
dev-aravind commented 1 year ago

The expected data format for type is different. "type": [{"en":"Organization"},{"en":"MusicGroup"}]

saumier commented 1 year ago

@SuhailAliyar The type here is a URI (the schema: prefix is defined in the @context) and I can't change that part of the JSON-LD. This is to indicate if the entity is a schema:Person or schema:Organization. In the example organization there is also a subType called schema:MusicGroup which is a type of schema:Organization. I can make a change to the SPARQL used in Artsdata to only select the top most type. This would force it to always be schema:Person or schema:Organization. I think having the type is critical for Aggregator to create the right type of entity in Footlight (correct me if I am wrong).

This would maintain the status-quo like is it currently working. { "uri": "http://kg.artsdata.ca/resource/K10-580", "type": "Organization", "name": { "fr": "Bleu Jeans Bleu", "en": "Bleu Jeans Bleu" } }

{ "uri": "http://kg.artsdata.ca/resource/K5-21", "type": "Person", "name": { "fr": "Étienne Coppée" } }

saumier commented 1 year ago

@SuhailAliyar Please confirm the JSON above is the current approach and then I will make the change I suggested to only return the top class of Person or Organization.

saumier commented 1 year ago

@dev-aravind It is difficult for Artsdata to filter out entity types when more than one type is explicitly stated for the entity. In this case the organization Blue Jean Bleu was explicitly claimed to be type Organization and type MusicGroup. I think this will need to be addressed in the Aggregator so that entities with more than one type will load in Footlight. It is OK to drop the extra types that are not recognized by Footlight CMS. The aggregator can count on receiving the top level entity type: Organization or Person or Place or Event. I can't think of any other types that we load directly with Aggregator. In the future we will add CreativeWork.

sahalali commented 10 months ago

@saumier Lets discuss this during todays standup.

sahalali commented 10 months ago

As we discussed during the standup, the type with an array should not break the aggregator. I think we can remove type from organization (for now) before passing to footlight.

dev-aravind commented 9 months ago

@saumier I couldn't fetch the organization that you mentioned in the description from artsdata to aggregator

dev-aravind commented 9 months ago

@saumier The coding part of this issue was resolved in #572 but as the event was last modified by @troughc, aggregator cannot modify it. This can be fixed if @troughc could run the co-motion aggregator from her footlight account.

saumier commented 9 months ago

Here is the link http://staging.cms.footlight.io/dashboard/63457978637093005071a053/events/add-event/644139d77a2993006450b3bb

saumier commented 9 months ago

@dev-aravind I ran into a bug "Request failed with status code 400 " running the aggregator with sync events. So I am blocked trying to test this issue. I ran Aggregator locally using the latest main branch (b101454) running node v16.19.1

http://localhost:3033/events/sync?footlight-base-url=https%3A%2F%2Fstaging.api.cms.footlight.io&calendar-id=signe-laval&source=http%3A%2F%2Fkg.artsdata.ca%2Fculture-creates%2Ffootlight%2Fco-motion-ca&batch-size=5&mapping-url=http%3A%2F%2Fraw.githubusercontent.com%2Fculturecreates%2Ffootlight-aggregator%2Fmain%2Fdata%2Fco-motion-cms-mapping.json

Fetching taxonomies
Unable to fetch URL https://staging.api.cms.footlight.io/taxonomy?include-concepts=true&taxonomy-class=EVENT Error ::, AxiosError: Request failed with status code 400
[Nest] 17769  - 09/12/2023, 10:53:34 AM   ERROR [ExceptionsHandler] Cannot read properties of undefined (reading 'map')
saumier commented 9 months ago

@dev-aravind @sahalali Can we add a single test in Aggregator for this fix? I would like to see a test with the JSON input for the Organization (we can Use Blue Jean Bleu as a text fixture) and the expected output that will be sent to CMS API.

The input will include

...
"type": [
        "Organization",
        "MusicGroup"
      ]

The expected output will have

...
"type": "Organization"

We don't need to add all cases now, but it would be much easier to test that running the aggregator which takes a long time.

Let me know what you think.

dev-aravind commented 9 months ago

@saumier The unit tests are up to be merged with production now.

saumier commented 9 months ago

@dev-aravind Looks good except I am wondering why you didn't add --detectOpenHandles to the package.json? Is there a reason?

This line is printing in my console each time I run the tests: Have you considered using --detectOpenHandles to detect async operations that kept running after all tests finished?

saumier commented 9 months ago

@dev-aravind I am closing this issue but please reply to my question above. Thx.

dev-aravind commented 9 months ago

@saumier We were actually not using detectOpenHandles in the OpenAPI unit tests. This is caused by the async operations that are running in the test. I've actually tried to fix this issue when we had the problem where the Open-API unit tests were failing in staging and decided to not include --detectOpenHandles as I suspected it was breaking change.