Added SecurityCompareConnector #69
Conversation
DJTechnoo
requested review from
pawelfus,
jedrzejruta,
bm64 and
kamil-musialowski
There was a problem hiding this comment.
Overall looks really solid, I like the concept of defaultly pulling TrailingPerfomance 🎉
Q: When pulling assetAllocations and other types, what about the not-classified number? In XRay, we put it into the column name which isn't ideal, but we need to have this information somewhere, perhaps in the metadata? 🤔
| name: typeMapping[item['AssetAllocations_Type_' + id]], | ||
| y: item['AssetAllocations_MorningstarEUR3_N_' + id] | ||
| })) | ||
| .filter(entry => entry.y !== 0) // Filter out entries where y is 0 |
There was a problem hiding this comment.
I think we have to keep all values, otherwise we have mismatching colors in series (compare cash):
| } | ||
|
|
||
|
|
||
| const connector = new HighchartsConnectors.Morningstar.SecurityCompareConnector({ |
There was a problem hiding this comment.
Let's move the connector config under Dash ataPool's config
| .map(item => ({ | ||
| name: typeMapping[item['AssetAllocations_Type_' + id]], | ||
| y: item['AssetAllocations_MorningstarEUR3_N_' + id] | ||
| })) |
There was a problem hiding this comment.
I think we can resign from this. Use dataLabels.format to apply the translation and columnAssignment to choose the right columns for charts
There was a problem hiding this comment.
Directory name:
Let's follow the convention, and since we use Dash here, the directory name should be dashboards-...
There was a problem hiding this comment.
I still believe this might be optimized. SecurityDetails parses one security. This one parses multiple, just using a bit different code. However under the hood it's the same:
const securityDetails = json[0]; // Details
...
vs
for (let i = 0; i < json.length; i++) {
const securityCompare = json[i], // Compare
...
The way I see it (please correct me if I'm wrong!):
- use only exiting TrailingConverter
- if JSON contains more than one security, then column names are
<name>_MSID, if just one, then just<name> metadata.idsandmetadata.isinsare set for multi-security response. For one security, setmetadata.idandmetadata.isin
There was a problem hiding this comment.
Note: I didn't check other converters, but perhaps the same can be solution can be used?
Added
SecurityCompareConnector.