openactive / implementation-tracker

Activation Issue Tracker for OpenActive Implementations
1 stars 2 forks source link

England Athletics feedback #168

Closed nickevansuk closed 1 year ago

nickevansuk commented 6 years ago

RPDE Validation

The beta RPDE validator errors are accurate: https://validator.openactive.io/rpde?url=https://api.runtogether.co.uk/odi/public/courses

Model Validation

The beta Model validator errors are accurate, with one exception (see below): https://validator.openactive.io/?url=https%3A%2F%2Fapi.runtogether.co.uk%2Fodi%2Fpublic%2Fcourses&version=2.0

Modelling Feedback

In the current endpoint, the URLs do not match the objects they are attached to, which suggests an issue with the underlying mapping of entities.

V2.0 includes SessionSeries and ScheduledSession to clarify these mappings, and the example below shows how to apply these while fixing the existing mapping in the process. See the Candidate Specification for more information.

Using correct Event types

In V2.0 use SessionSeries and ScheduledSession instead of just Event, as follows:

{
  "type": "SessionSeries",
  "url": "https://groups.runtogether.co.uk/PrudhoePlodders/Runs/Summary/db1aa011-9b65-4732-b10c-9020ecc2e2c1",
  "subEvent": [
    {
      "type": "ScheduledSession",
      "startDate": "2018-03-26T18:30:00+01:00",
    }
  ]
}

Summary of model feedback

Example

Each comment included in the example below should be examined, and the object structure compared with the current endpoint to understand where changes need to be made.

{
  "items": [
    {
      "id": "45a74724-7cc1-47c1-a900-010aea5f1e01",
      "state": "updated",
      "kind": "SessionSeries",
      "modified": 1535550756960,
      "data": {
        "@context": [
          "https://openactive.io/",
          "https://openactive.io/ns-beta"
        ],
        "type": "SessionSeries",
        "identifier": "db1aa011-9b65-4732-b10c-9020ecc2e2c1",
        "url": "https://groups.runtogether.co.uk/PrudhoePlodders/Runs/Summary/db1aa011-9b65-4732-b10c-9020ecc2e2c1",
        "description": "Interval running to build speed, for all abilities",
        "name": "Speedy Mondays",
        "level": [
          "All Levels"
        ],
        "genderRestriction": "https://openactive.io/NoRestriction",
        "ageRange": {
          "type": "QuantitativeValue",
          "minValue": 0,
          "unitCode": "ANN"
        },
        "isCoached": false,
        "eventStatus": "https://schema.org/EventScheduled",
        "beta:distance": { /* include distance */
          "type": "QuantitativeValue",
          "minValue": 0,
          "maxValue": 5,
          "unitCode": "KMT"
        },
        "eventSchedule": [  /* include as much info about the schedule as possible here */
          {
            "type": "PartialSchedule",
            "startDate": "2017-01-01",  /* ignore if not available */
            "endDate": "2017-12-31",  /* ignore if not available */
            "repeatFrequency": "P1W",  /* as implied by website */
            "byDay": [
              "https://schema.org/Monday"  /* as shown on website */
            ],
            "startTime": "06:30"  /* as shown on website */
          }
        ],
        "activity": [
          {
            "id": "https://openactive.io/activity-list#72ddb2dc-7d75-424e-880a-d90eabe91381",
            "inScheme": "https://openactive.io/activity-list",
            "prefLabel": "Running",
            "type": "Concept"
          }
        ],
        "category": [
          "Runs"
        ],
        "isAccessibleForFree": true, /* should be set if all Offers are zero price */
        "location": {
          "type": "Place",
          "address": { /* ensure this contains as much in is available, or do not include the  */
            "type": "PostalAddress",
            "streetAddress": "Raynes Park High School, 46A West Barnes Lane",
            "addressLocality": "New Malden",
            "addressRegion": "London",
            "postalCode": "NW5 3DU",
            "addressCountry": "GB"
          },
          "geo": {
            "type": "GeoCoordinates",
            "latitude": 54.956489,
            "longitude": -1.856582
          },
          "identifier": "73d21750-7c0d-4f41-a3ed-2bc057ca65b6",
          "name": "Regents Drive"
        },
        "image": [  /* cover image of run - include full image and thumbnail used on search page */
          {
            "type": "ImageObject",
            "url": "https://sportlabs.blob.core.windows.net/ddea7f82-a225-460e-969f-6ba4c3517e47/Coaching/0cbaedff-81d3-4139-8afa-0ec22e5d5837/Photos/d28cc6aa-2a40-4e9f-935d-ba49205cc0c0.jpg"
          }
        ],
        "organizer": {
          "type": "Organization",
          "id": "https://groups.runtogether.co.uk/PrudhoePlodders/Runs",
          "url": "https://groups.runtogether.co.uk/PrudhoePlodders/Runs",
          "name": "Prudhoe Plodders",
          "description": "Prudhoe Plodders was born from a volunteer organised Couch to 5k programme to help people who want to improve their running fitness and stamina. We now have over 100 members, but have still retained our friendly, all inclusive membership where all abilities are welcomed.\n\nWe meet Monday nights at 6:30 pm and Friday nights at 6pm, at Regent's Drive, Prudhoe and run in an enclosed area so no one can get left behind, regardless of their pace or fitness. All abilities are welcomed and catered for and the sessions are written by a Level 2 Coach in Running Fitness, and delvered by a qualified coach or run leader.\n\nSessions can be booked on an individual basis at £2.00 per session, or you may join Plodders and all our sessions are included in your membership which is £15.00 per year. For information about joining, please email prudhoeplodders@outlook.com",
          "image": [  /* cover image of group - this is currently the image of the run*/
            {
              "type": "ImageObject",
              "url": "https://sportlabs.blob.core.windows.net/ddea7f82-a225-460e-969f-6ba4c3517e47/Coaching/0cbaedff-81d3-4139-8afa-0ec22e5d5837/Photos/d28cc6aa-2a40-4e9f-935d-ba49205cc0c0.jpg"
            }
          ],
          "sameAs": [ /* facebook link to group */
            "http://facebook.com/prudhoeplodders"
          ]
        },
        "offers": [ /* it doesn't look like all of theses offers are available online, only include those available online */
          {
            "type": "Offer",
            "price": 0,
            "priceCurrency": "GBP",
            "identifier": "COURSE",
            "name": "Full course"
          },
          {
            "type": "Offer",
            "price": 0,
            "priceCurrency": "GBP",
            "identifier": "SINGLE",
            "name": "Single session"
          }
        ],
        "subEvent": [
          {
            "type": "ScheduledSession",
            "category": [
              "Standard"
            ],
            "url": "https://groups.runtogether.co.uk/PrudhoePlodders/Runs/Summary/905981db-d633-4eed-bd1d-0042f1c8efbd",
            "isCoached": false,
            "startDate": "2018-03-26T18:30:00+01:00",
            "endDate": "2018-03-26T19:00:00+01:00",
            "duration": "PT30M",
            "offers": [
              {
                "type": "Offer",
                "price": 0,
                "priceCurrency": "GBP",
                "identifier": "SINGLE",
                "name": "Single session"
              }
            ],
            "eventStatus": "https://schema.org/EventScheduled",
            "maximumAttendeeCapacity": 40,
            "remainingAttendeeCapacity": 20,
            "identifier": "905981db-d633-4eed-bd1d-0042f1c8efbd"
          },
          {
            "type": "ScheduledSession",
            "category": [
              "Standard"
            ],
            "url": "https://groups.runtogether.co.uk/PrudhoePlodders/Runs/Summary/4136e3eb-a86d-41f1-89d0-00b38ad5b00b",
            "isCoached": false,
            "startDate": "2018-04-30T18:30:00+01:00",
            "endDate": "2018-04-30T19:00:00+01:00",
            "duration": "PT30M",
            "offers": [
              {
                "type": "Offer",
                "price": 0,
                "priceCurrency": "GBP",
                "identifier": "SINGLE",
                "name": "Single session"
              }
            ],
            "eventStatus": "https://schema.org/EventScheduled",
            "maximumAttendeeCapacity": 50,
            "remainingAttendeeCapacity": 20,
            "identifier": "4136e3eb-a86d-41f1-89d0-00b38ad5b00b"
          }
        ]
      }
    }
  ],
  "next": "https://api.runtogether.co.uk/odi/public/courses?afterTimestamp=1535550756960&afterID=efe5f0b2-498f-4daf-bf37-002fcf5562fd",
  "license": "https://creativecommons.org/licenses/by/4.0/"
}
peter-dolkens commented 6 years ago

Hey @nickevansuk

Just going over this, and the spec at https://www.openactive.io/modelling-opportunity-data/EditorsDraft/#describing-events-code-schema-event-code-

In particular, this bit:

The validator is missing one check:

image property should always be an array. This should be corrected in the feed.

In it, you state the schema:image property on schema:Event is an optional Array of schema:ImageObject

schema:Event inherits from schema:Thing which specifies image as schema:ImageObject OR schema:URL - neither of these appear to be arrays.

Can you confirm this, as it appears to break compliance with schema:Event, which is fine, but it should then be moved to the oa namespace.

Cheers

Pete

nickevansuk commented 6 years ago

Hi @dolkensp,

No problem: the issue is that schema.org is loose in that it allows all properties to contain a single value or array of values. The OpenActive V2 spec tightens this up by defining which properties should be arrays and which should not, to create additional consistency in the data.

You will note the "Type" column in the section of the spec you are referring to is new in V2.

I've just looked at the validator again and there are further issues with errors to note (the errors previously reported above have now been resolved):

I've also updated the example in the previous comment to match the latest spec.

Thanks,

Nick

nickevansuk commented 6 years ago

Also any feedback on the spec very welcome @dolkensp as it's planned to be published soon :)

nickevansuk commented 6 years ago

@dolkensp looking at https://api.runtogether.co.uk/odi/public/courses there are still a number of issues. Please do check the example above throughly against the output from the endpoint.

Example issues:

peter-dolkens commented 6 years ago

I also see the Organizer.Image.Urls haven't been reconstructed fully

image

I'll send this back to the team to finish off.

peter-dolkens commented 6 years ago

Hey @nickevansuk

Can you let me know a few things:

Cheers

nickevansuk commented 6 years ago

Hi @dolkensp, not 100% sure which URLs you're referring too, so hopefully the below is helpful:

List of URLs

URL A - Organization

Good point: The landing page for an Organizer(Group) would be best to use here.

https://groups.runtogether.co.uk/PrudhoePlodders should be mapped to an Organization which is the organizer of the SessionSeries.

The group name is "Prudhoe Plodders", they are the "organizer" of the event. We would consider the below to be an "organizer page".

screen shot 2018-09-20 at 23 29 06

URL B - SessionSeries

https://groups.runtogether.co.uk/PrudhoePlodders/Runs/Summary/6d05f462-a97c-416e-89c7-e2fb32ee8438 should be mapped to a SessionSeries which is the root of the RPDE feed item

This is a series of sessions (i.e. SessionSeries) (you can see in the screenshot below there's some eventSchedule data on the right-hand side, for example, along with genderRestriction and distance info. This is what a SessionSeries object represents, the whole page below:

screen shot 2018-09-20 at 23 31 10

Missing URLs

ScheduledSession

https://groups.runtogether.co.uk/PrudhoePlodders/Runs/Summary/6d05f462-a97c-416e-89c7-e2fb32ee8438 contains a list of individual sessions, which should be mapped to an array of ScheduledSession, within subEvent of this SessionSeries.

So each row in the table on the page below represents an individual ScheduledSession, which all should be listed via an array within subEvent of this SessionSeries.

As each row of the table below does not have an individual URL, it's ok to leave out the ScheduledSession URL.

screen shot 2018-09-20 at 23 33 46

Example with URLs included

{
  "type": "SessionSeries",
  "url": "{{URL B}}",
  "organizer": {
    "type": "Organization",
    "id": "{{URL A}}",
    "url": "{{URL A}}"
  },
  "subEvent": [
    {
      "type": "ScheduledSession",
      "startDate": "2018-09-23T09:30:00+01:00"
    }
  ]
}
peter-dolkens commented 6 years ago

Hey @nickevansuk

URL B - SessionSeries

https://groups.runtogether.co.uk/PrudhoePlodders/Runs/Summary/6d05f462-a97c-416e-89c7-e2fb32ee8438 - This is actually URL C - that guid on the end is the identifier for the individual ScheduledSession.

That's what I was trying to highlight in my previous comment. The URL is actually URL C, but the page we display looks like something you'd expect from a URL B.

Clubspark has no equivalent url for URL B, so the best we can do is point at a URL C

As such, we can pick the first URL C and use that in place of URL B at the moment - but if there's no scheduled sessions, or one of the sessions is removed, then this url will change.

I hope that's easier to follow?

Also, should we be showing past events/offers?

nickevansuk commented 6 years ago

Oh I see, @dolkensp. That's helpful - thanks!

As long as the "modified" property of the item is updated when new events appear or disappear within the object, or when the URL changes, then everything will work fine.

The past events/offers within a SessionSeries are not required.

If there's no scheduled sessions I'd suggest just setting the whole item to "deleted" (assuming that this will also update "modified", as without scheduled sessions the SessionSeries is not useful).

You could potentially use the last URL C for the SessionSeries to maximise the chances of it being valid near the start time of the session. From what you're saying though, all URL Cs produce identical pages?

ykkp2000 commented 6 years ago

Hi @nickevansuk ,

We've made changes to the api, could you have a quick look at this feed https://test-api.runtogether.co.uk/odi/public/courses and confirm it's ok?

We're planning to deploy this to production on Monday.

Thanks,

Kevin

nickevansuk commented 6 years ago

Hello @ykkp2000, sorry for the delay in reply.

Running this example through the validator there are still a number of errors. See https://validator.openactive.io/?url=https://test-api.runtogether.co.uk/odi/public/courses. There's also data missing compared with the data available on the website and in the example at https://github.com/openactive/activation/issues/168#issue-355730000

To make this most efficient we'll go through and enumerate the issues for you this afternoon, which should give you a clear list to work through and get this to completion.

Thanks,

Nick

peter-dolkens commented 6 years ago

Hey @nickevansuk

Regarding this error:

Could not validate unitCode property because the "@context" value "https://www.openactive.io/ns-beta" did not return a valid JSON response. Please check that it contains a JSON document in the format described in the specification.

Additionally, please check the "@context" property in the root object to ensure all values are valid.

The correct way to reference the OpenActive contexts is using these URLs:

"https://openactive.io/" (always required)
"https://openactive.io/ns-beta" (only required if using beta: properties)

Checking the ns-beta beta:distance property, I see

{
    id: "beta:distance",
    type: "Property",
    label: "distance",
    comment: "The distance of a run, cycle or other activity. Must also include units.",
    domainIncludes: [ "schema:Event" ],
    rangeIncludes: [ "schema:QuantitativeValue" ],
    githubIssue: "https://github.com/openactive/ns-beta/issues/3"
}

schema:QuantitativeValue should cover this as per https://schema.org/QuantitativeValue

Not quite sure I understand why we're seeing this error?

nickevansuk commented 6 years ago

You're using https://www.openactive.io/ns-beta, should be https://openactive.io/ns-beta. Remove the www.

peter-dolkens commented 6 years ago

Hi again @nickevansuk

Required property postalCode is missing from PostalAddress.

A full example looks like this:

"postalCode": "EC2A 4JE"

This, and the other 3 required properties seem overly restrictive.

I've implemented it, but I'm concerned that many venues may have only provided Address + Town or Address + Postcode or similar, resulting in many addresses being removed from the API, reducing its usefulness.

peter-dolkens commented 6 years ago

@nickevansuk

Activity "Running" was found in the activity list "https://openactive.io/activity-list", but the "id" did not match.

The correct "id" is "http://openactive.io/activity-list/#72ddb2dc-7d75-424e-880a-d90eabe91381".

Everywhere else references https://openactive.io, but this one wants http?

Can this be ignored? I'm assuming this will change soon.

nickevansuk commented 6 years ago

That's right re: activity @dolkensp see the second heading in https://github.com/openactive/activation/issues/168#issue-355730000

Regarding addresses: it's best practice not to remove data when getting the validator to conform, either fields or records. There's an open issue for this (https://github.com/openactive/modelling-opportunity-data/issues/193), so suggest for now just include as much address info as you have.

nickevansuk commented 6 years ago

Hi @dolkensp @ykkp2000 I'm going through this to enumerate the remaining issues at 5pm, so if you push your latest version to test before then I can do that against the work you've done today.

Thanks,

Nick

peter-dolkens commented 6 years ago

Hi @nickevansuk

Should all be good for you to review. I've removed "empty" addresses, but kept any that have at least partial address information.

Currently, the validator only returns that 1 http/https error, everything else is a tip or warning.

I've left URL Cs on the ScheduledSession entities, as there is potentially work coming up for EA and LTA which may make these more useful going forwards. In the meantime, the SessionSeries URL is set to be the latest URL C available.

Let me know if there's any other showstoppers, but I think this should satisfy all your requirements for now.

nickevansuk commented 6 years ago

Great thanks @dolkensp - it's looking much better!

Final review here: https://github.com/openactive/activation/projects/7

Please do drop everything into QA when done, so we can move them into Done and sign it off.

Sounds good regarding URL C

Great progress!