mrc 4959 Refactor metadata for filters component

[M-Kusumgar]

Build isn't currently passing but the code should be fine, going to sort out some test failures today but does this approach make sense. Essentially we want of this type (from the calls I have had with you guys):

type Warning = {
    locations: string[],
    text: string
};

type FilterId = string

type StateFilterId = string

type FilterType = {
    id: FilterId,
    column_id: string,
    options: FilterOption[]
}

type FilterRef = {
    filterId: FilterId,
    label: string,
    stateFilterId: string
}

type PlotSettingEffect = {
    setFilters?: FilterRef[],
    setMultiple?: StateFilterId[],
    setFilterValues?: Record<StateFilterId, string[]>
}

type PlotSettingOption = {
    id: string,
    label: string,
    effect: PlotSettingEffect
}

type PlotSetting = {
    id: string,
    label: string,
    options: PlotSettingOption[]
}

type PlotSettingsControl = {
    defaultFilterTypes: FilterRef[],
    plotSettings: PlotSetting
}

type NewMetadata = {
    // this has all possible filter types i.e.
    // area_id, area_level, calendar_quarter, indicator, sex, age_group
    filterTypes: FilterType[],

    // this has extra info about indicators such as scale, format, accuracy, but
    // this will have the same length as the indicator filter type options
    // we just dont need everything in here in the filterTypes
    indicators: Indicator[],

    plotSettingsControl: {
        choropleth: PlotSettingsControl,
        barchart: PlotSettingsControl,
        table: PlotSettingsControl,
        bubble: PlotSettingsControl
    },

    warnings: Warning[]
}

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.51%. Comparing base (038c0d9) to head (ce610d8).
Report is 64 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #491      +/-   ##
==========================================
+ Coverage   98.49%   98.51%   +0.02%     
==========================================
  Files          29       29              
  Lines        2389     2429      +40     
==========================================
+ Hits         2353     2393      +40     
  Misses         36       36              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[M-Kusumgar]

have to define the package as hintr because for some reason traduire cannot get package from context, this is something that is also in naomi

[r-ash]

Thanks, this looks good after a quick skim through but I will look properly on Monday.

Just a couple of surface level things. Could you also bump the version number in the DESCRIPTION file and add a news item in NEWS.md

[r-ash]

I should have done this in naomi but maybe worth putting a note at the top here that this is needed because when running tests it sometimes picking up the package environment as testthat instead of hintr as it should.

[M-Kusumgar]

done

[r-ash]

I think we do not need this? with_mocked_bindings provided by testthat.

[r-ash]

Can you move these helper functions into helper-hintr.R. Functions you want to source before running tests are in helper- by convention and then testthat will source them as part of its setup.

[r-ash]

Ah I see using this library to remove the namespacing when creating a mock. In general I always use namespaced calls in R packages as they affect the users environment which can cause problems. This is fine here as just sourced in tests, but perhaps worth moving into tests/testthat.R with the other library calls.

[M-Kusumgar]

okay makes sense! will move the using library bit to test/testthat.R and helper functions to helper-hintr.R!

[EmmaLRussell]

Can't really comment much on the R code, but the example metadata in the description of this PR is really useful, particularly the comments! I wonder if you should include this somewhere (in the README?), ideally with additional comments covering what the difference is between plot settings, plot settings effect and plot setting controls, what the different effect fields mean, what stateFilterId means etc... These probably all seem really obvious to you, but didn't to me, and maybe won't to some approaching the metadata for first time or after a break!

In PlotSettingEffect, am I right in remembering that setFilters are just the filters to include as available when the effect is selected, setMultiple are the subset of those filters which will allow multiple values, and setFilterValues are the default filter values which the effect will select when it's first loaded (and these default values won't be changeable by the user if the filter isn't also in setFilters?). I think I find the set.. prefixes a bit confusing, but I guess it's to do with this being an 'effect' and what the effect does... so I think it will be fine with a little bit of docs.

What does defaultFilterTypes in PlotSettingsControl do? Is that the filters you get if the plot doesn't actually have any settings defined e.g. choropleth?

Also, in your description example plotSettings is defined as a single PlotSetting, but that should be multiple (you can have multiple dimensions of settings on a single plot)?

[EmmaLRussell]

Suggested change

	"plotSettingsControl": {
	"plotSettingsControls": {

[M-Kusumgar]

documentation thoroughly
validation on backend for defaultFilterTypes to contain reference by other effects
only have default effect and not plotsettings in choropleth for example

[r-ash]

Superseded by #493