refactor getSupportedChartTypes() to clarify the rules by chartType #2359
Conversation
…nstead of using procedural code
server/src/termdb.server.init.js
Outdated
| supportedChartTypes[r.cohort].add('cuminc') | ||
| if (numericTypes.has(r.type)) numericTypeCount[r.cohort] += r.samplecount | ||
| } | ||
| const isSupportedChart = { |
There was a problem hiding this comment.
this is a good design. it can be improved by removing profile charts. this should only define general chart types that can be determined by term type availability. profile charts and any other special purpose charts are only defined in their own ds.cohort.specialCharts[]. and will be added to supportedChartTypes as-is.
There was a problem hiding this comment.
So that would mean that any chartType that's listed in specialCharts, is not meant to be be listed in hiddenCharts, right? In other words, if it's listed in specialCharts, just ignore hiddenCharts for it.
There was a problem hiding this comment.
correct. special charts will simply be added and will not be explicitly coded here, this simplifies logic
There was a problem hiding this comment.
Maybe we can have a method that does a switch with cases for the charts with more complex logic and a default behaviour, something like:
isSupported(chart)
{
switch(chart):
...
default:
return specialCharts.includes(chart) && !hiddenCharts.includes(chart)
}
Description
... instead of using procedural code. Closes #2229
Tested with TermdbTest only, other datasets will need to be migrated in
sjpprepo. The chart buttons in localhost should match the ones in prp1 (11 charts).Tested with http://localhost:3000/testrun.html?name=*.unit and
cd client && npm run test:integration.Checklist
Check each task that has been performed or verified to be not applicable.