Add plan prices to films in kibble 6688

[matthewthomsonnz]

This PR makes it so in the template, we see the prices of all the plans that each film belongs to. e.g. we could now go site.Films[0].Prices.PlanPrices (not that we need to)

The actual reason for this PR is:

• We're rolling out a video feed for every client (example).
• In the feed, we need to get the lowest price for each film, including plan prices. Or lowest plan price if film price is zero.
• This PR is just a step to this. (broken out in order to make pull requests smaller. The final step will be to add a method to film.Prices called getLowestPriceIncludingPlanPrices()

AB#6688

[stajs]

Just a thought: I realise this is a complaint on existing code but feels like the modelling isn't very nice - plan.Prices.Prices and film.Prices.PlanPrices are a bit clunky. I'm thinking maybe we should remodel a breaking change for v1 so film.Prices is aware of the origin (plan/tvod/other?) with whatever properties/methods needed for getting lowest price.

[matthewthomsonnz]

Just a thought: I realise this is a complaint on existing code but feels like the modelling isn't very nice - plan.Prices.Prices and film.Prices.PlanPrices are a bit clunky.

Agreed. Changed to item.PriceInfo.Prices and item.PriceInfo.PlanPrices

I'm thinking maybe we should remodel a breaking change for v1 so film.Prices is aware of the origin (plan/tvod/other?) with whatever properties/methods needed for getting lowest price.

If I understand correctly, instead of

"Prices":[{...}{...}],"PlanPrices":{"/plan/1":[{...}],"/plan/2":[{...}]}

You propose: "Prices":[{"Ownership":"buy","Quality":"hd","Price":"0","origin":"film"}, {"Ownership":"buy" ..."origin":"plan"}],"

I'm not 100% sure this reads better to me, but will change it if you reckon its a better approach?

[stajs]

Sorry if I've led you astray on this - it was more a thought experiment rather than an actual proposal, but yes my half-formed thought was item.Prices having everything rather than separate item.PriceInfo.Prices and item.PriceInfo.PlanPrices - the unformed other half was the implementation.

I think the name change is a minor improvement, however it's a breaking change by itself so I wouldn't make it until v1 anyway.

[matthewthomsonnz]

Sorry if I've led you astray on this - it was more a thought experiment rather than an actual proposal, but yes my half-formed thought was item.Prices having everything rather than separate item.PriceInfo.Prices and item.PriceInfo.PlanPrices - the unformed other half was the implementation.

I think the name change is a minor improvement, however it's a breaking change by itself so I wouldn't make it until v1 anyway.

Oh yep, that would definitely be better for v2, and would just need currency added to each price.

I've reverted the item.PriceInfo.Pricesanditem.PriceInfo.PlanPrices change

[stajs]

@matthewthomsonnz Don't forget changelog.

[matthewthomsonnz]

@matthewthomsonnz Don't forget changelog.

Changelog updated

[matthewthomsonnz]

@additiverich I believe this PR can be approved/merged if you're happy with it. It will automatically utilise the upcoming optimisation changes to FindFilmBySlug and FindPlanBySlug (in this separate WIP branch).

We won't roll prices out to all clients until the optimisation changes are merged.

[matthewthomsonnz]

Is there an example of how this would get used in a template?

I was wondering if a helper method made sense, getLowestAvailablePrice()

@sarge This kibble branch here is what I had in mind (note still very draft but works)

The template needs to know where to put the last json comma (there could be multiple zero price films at end of loop), so a method needs to be called before the loop, to filter out any films with a zero/free price.

Get lowest price in the template then looks like this:

{{ site.Films.CalculateLowestAvailablePricesHideFree() }}
{{range index, item := site.Films }}
  slug is:{{item.Slug}}
  Lowest available price : {{ item.Prices.LowestAvailablePrice }}
--------------------------------------
{{end}}

[sam-shift72]

Pricing rules are too dynamic to be useful in templates so I don't think this is hugely valuable.

If we want to roll out videos.json to other clients we should probably just build that as a feature in Kibble because constructing complex JSON from a Jet template is unmaintainable.