-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix group assignment in votes matrix export #1944
base: edge
Are you sure you want to change the base?
Conversation
@ballPointPenguin , as per @colinmegill request, flagging the ``.members function getGroupId(pid: number) {
if (!baseClusters || !groupClusters) {
return undefined;
}
// First find which base cluster contains this participant
const baseClusterIds = baseClusters.id;
const baseClusterMembers = baseClusters.members;
let baseClusterId = -1;
for (let i = 0; i < baseClusterIds.length; i++) {
if (baseClusterMembers[i].includes(pid)) {
baseClusterId = baseClusterIds[i];
break;
}
}
// If we found a base cluster, check which group contains it
if (baseClusterId !== -1) {
for (const group of groupClusters) {
if (group.members.includes(baseClusterId)) {
return group.id;
}
}
}
return undefined;
} The code runs as is, but we do get a linter error. |
I confirm that:
2/ this pull-request as it is fixes the bug (even with the typescript warning discussed above -- there must be a tolerance in typescript :) ):
Thanks @metasoarous for the clear report, and @colinmegill for coding the fix. |
@ballPointPenguin Ah, I think I know why Colin's code works in spite of the linter error. His code respects the actual structure of the JSON, as illustrated by @metasoarous in the screenshots here: #1930 (comment) That's why it works. So I suspect it's only somewhere the Typescript type definition for |
So I believe (but haven't tested) that the right type should be:
Actually, give me a second and I'll test the export with that modification. |
Yup, confirmed, with the modified type I describe above the linter is happy and the export is still good :)
I don't know how to edit this PR as I don't have push access to the polis repo, so I'll let you do it :) |
* fix ts error in export.ts * improved exports.ts and pca.ts to better handle group-id mapping, proper types, logging, and general code cleanup. * Fix and improve export test * fix server build
Updated this PR with cleanup and fixes. |
check base cluster for pid, check group for base cluster