search

JackHaK / TMF

0 stars 0 forks source link

Related Courses is a Free Text box in Administrate - #21

Open JazHaK opened 6 years ago

JazHaK commented 6 years ago

do we want to get more structured on the web site ??? Should we use Categories?

seanthepottingshed commented 6 years ago

@joshgosselin

What are your thoughts on the above? As it stands the website is expecting a series to course ids to populate related courses.

joshgosselin commented 6 years ago

@seanthepottingshed are we setting a specific value of related courses (2, 3,5 ???)

My thoughts are that we can default to courses from the same category. But maybe with the functionality that they can define a specific course if appropriate.

Not sure what your thoughts are for the best implementation of this.

Could we allow the GTA to enter the ID into the free text field in administrate and parse that in the integration tier. And populating any remaining slots with courses based on category. Obviously they need to get the CSV ids structure correct, but does allow control from Administrate.

seanthepottingshed commented 6 years ago

@joshgosselin

We aren't setting a specific value of Related Courses, each Course when imported will have a reference to the integration_tier_id, the relationship to Related Courses would be via integration_tier_id's.

It's easy enough to amend the current logic to fetch 5 random courses from the categories that the course belongs to on our side ( we'd previously agreed that 5 should be the limit for related courses displayed in a course details page ), and as you've suggested allow other integration_tier_id's to be entered via Adminstrate and populate the remaining 5 as before.

joshgosselin commented 6 years ago

Yep make sense to it all to be managed by the integration tier.

@JackHaK does this make sense to you:

PeteHaK commented 6 years ago

The integration tier uses Administrate IDs so this will work – and actually the Administrarte screens use Course IDs to navigate so using course ids in the related courses fields comma delimited will work fine.

We’re gonna struggle to support this channel today as we’re all busy but queue up the issues and we’ll turn them around overnight if we can.

Pete

From: Sean Davidson notifications@github.com Sent: 15 October 2018 10:32 To: JackHaK/TMF TMF@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [JackHaK/TMF] Related Courses is a Free Text box in Administrate - (#21)

@joshgosselin https://github.com/joshgosselin

We aren't setting a specific value of Related Courses, each Course when imported will have a reference to the integration_tier_id, the relationship to Related Courses would be via integration_tier_id's.

It's easy enough to amend the current logic to fetch 5 random courses from the categories that the course belongs to on our side ( we'd previously agreed that 5 should be the limit for related courses displayed in a course details page ), and as you've suggested allow other integration_tier_id's to be entered via Adminstrate and populate the remaining 5 as before.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JackHaK/TMF/issues/21#issuecomment-429773928 , or mute the thread https://github.com/notifications/unsubscribe-auth/Amr1EgdIsMdo_rMw-aPT3D6YxBZmjQQEks5ulFYogaJpZM4XZqzi . https://github.com/notifications/beacon/Amr1EseeXuaHq9CBoU0pLr49zU5Evtkrks5ulFYogaJpZM4XZqzi.gif

seanthepottingshed commented 6 years ago

@PeteHaK

Thanks, appreciated.

joshgosselin commented 6 years ago

No worries @PeteHaK, we have plenty to crack on with so as and whenever works for you.

PeteHaK commented 6 years ago

relatedCourses added as a simple JSON string array of course IDs. The administrate field adds HTML tags which we've removed on inflate - there is a chance that someone putting rubbish in Administrate will cause us an issue - lets see

seanthepottingshed commented 6 years ago

@PeteHaK @joshgosselin

Thanks for sorting out the relatedCourses this now works:

Is it possible to ensure that there are always 5 relatedCourses as per @joshgosselin comment above?

seanthepottingshed commented 6 years ago

@PeteHaK @JazHaK @JackHaK @joshgosselin

During the process of updating API integration, I've noticed that the following:

...has related course ids of 

[302, 303, 304, 305]

However there is no course with an id of 304:

PeteHaK commented 6 years ago

That was just test data – sorry. Unfortunately the data comes from Administrate as free text so there is no way we can validate the input.

There is a chance that GTA will do this so you will need to cater for IDs that don’t exist. Alternatively we could mark invalid courses on the way through the integration tier if easier.

seanthepottingshed commented 6 years ago

@PeteHaK

No worries, thanks for the clarification.

seanthepottingshed commented 6 years ago

I will continue to ignore related events without ids.