Skip to content

Conversation

@jake-bassett
Copy link
Contributor

Description

Some additional changes for Time Agnostic APIs. This fixes an issue with drilling into an API that is fetched in a list using includeInactive. We now include that property whenever querying for a single API. The second change here is to display null data correctly.

@jake-bassett jake-bassett requested a review from a team as a code owner February 18, 2021 22:19
@codecov
Copy link

codecov bot commented Feb 18, 2021

Codecov Report

Merging #612 (c36196d) into main (9dd42c5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #612   +/-   ##
=======================================
  Coverage   85.17%   85.18%           
=======================================
  Files         770      770           
  Lines       15905    15905           
  Branches     2048     2048           
=======================================
+ Hits        13547    13548    +1     
+ Misses       2324     2323    -1     
  Partials       34       34           
Impacted Files Coverage Δ
...ell/metric/metric-table-cell-renderer.component.ts 100.00% <ø> (ø)
...nents/entity-renderer/entity-renderer.component.ts 100.00% <ø> (ø)
...ery/entity/entity-graphql-query-handler.service.ts 56.00% <ø> (ø)
...tilities/formatters/numeric/display-number.pipe.ts 84.00% <100.00%> (+4.00%) ⬆️
...table/data-cell/metric/metric-table-cell-parser.ts 92.00% <100.00%> (ø)
...table/data-cell/entity/entity-table-cell-parser.ts 75.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9dd42c5...c36196d. Read the comment docs.

@github-actions

This comment has been minimized.

return cellData;
case 'object':
return cellData === null ? undefined : cellData.value;
return cellData === null ? undefined : cellData.value === null ? undefined : cellData.value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: cellData?.value ?? undefined

filters: [new GraphQlEntityFilter(request.id, request.entityType)]
filters: [new GraphQlEntityFilter(request.id, request.entityType)],
// If querying for a single API, then always want to includeInactive
includeInactive: request.entityType === ObservabilityEntityType.Api
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would apply always right? Even if we disabled this functionality elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Good point. It would. Not sure how to feature flag this cleanly. Open to suggestions.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@jake-bassett jake-bassett merged commit a15f0f4 into main Feb 19, 2021
@jake-bassett jake-bassett deleted the time-agnostic-apis branch February 19, 2021 23:23
@github-actions
Copy link

Unit Test Results

    4 files  ±0  236 suites  ±0   14m 16s ⏱️ -44s
849 tests ±0  849 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
853 runs  ±0  853 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit a15f0f4. ± Comparison against base commit 9dd42c5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants