Add custom filter tests with new samples and assertResponse fields #4998
Conversation
gblaih
force-pushed
the
clickhouse-new-custom-data-filter-tests
branch
from
34d2788
to
53d4605
Compare
gblaih
force-pushed
the
clickhouse-new-custom-data-filter-tests
branch
4 times, most recently
from
82f29d2
to
26fc0e2
Compare
gblaih
force-pushed
the
clickhouse-new-custom-data-filter-tests
branch
from
26fc0e2
to
32a4faf
Compare
src/shared/api/validation.ts
Outdated
| let legacyUrl = url.replace(/column-store\//, ''); | ||
| if (treatmentLegacyUrl[label]) { | ||
| legacyUrl = treatmentLegacyUrl[label](legacyUrl); | ||
| if (assertResponse && assertResponse[0] && assertResponse[0].attributeId) { |
There was a problem hiding this comment.
not sure i understand the necessity of this code up here. the clickhouse fetch should be entirely independent of the assertResponse, no?
There was a problem hiding this comment.
i used it to get the one attribute that is used in assertResponse. so the clinical data counts clickhouse fetch only fetches data for that attribute. then its an easy compare between that response and assertResponse.
src/shared/api/validation.ts
Outdated
| 0 | ||
| )} legacy: ${result.legacyDuration.toFixed(0)}` | ||
| ); | ||
| (!result.legacyDuration |
There was a problem hiding this comment.
try to avoid these duplicating logic (console.log(...)) like this for only a "little" thing like whether it has legacyDuration. better to introduce a variable that gets always gets printed and can sometimes be blank (e.g. when there's no legacyDuration).
There was a problem hiding this comment.
i do think it's a good idea to include the fact that there's an assertResponse in this console log here.
|
I will merge this in a new PR since there are conflicts |
No description provided.