Skip to content
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

Create foundation for a controller status history view #1402

Open
wants to merge 38 commits into
base: main
Choose a base branch
from

Conversation

kiahna-tucker
Copy link
Member

@kiahna-tucker kiahna-tucker commented Dec 16, 2024

Issues

The issues directly below are advanced by this PR:
#1377

Changes

1377

The following features are included in this PR:

  • Create an environment variable for the base /status endpoint URL.

  • Add a declaration file for control-plane types relevant to the /status endpoint.

  • Add a Status section to the Logs tab of the Details page which serves as the foundation for comprehensive entity status view.

  • Update the shared CardWrapper component to support a headerless display.

Tests

Manually tested

  • scenarios you manually tested

Automated tests

  • unit testing covered

Playwright tests ran locally

  • Admin
  • Captures
  • Collections
  • HomePage
  • Login
  • Materialization

Screenshots

Initialization

Hydration failed | Error returned by /status endpoint

pr_screenshot-1402-entity_status_foundation-error-hydration

Post-Initialization

Dashboard view

pr_screenshot-1402-entity_status_foundation-success-history

Code view

pr_screenshot-1402-entity_status_foundation-success-code

No controller status history returned by /status endpoint

pr_screenshot-1402-entity_status_foundation-success-no_history

Updating the view via the Refresh CTA | Table View

pr_screenshot-1402-entity_status_foundation-success-refreshing



@kiahna-tucker kiahna-tucker added the change:planned This is a planned change label Dec 16, 2024
@kiahna-tucker kiahna-tucker marked this pull request as ready for review January 14, 2025 14:14
@kiahna-tucker kiahna-tucker requested a review from a team as a code owner January 14, 2025 14:14
@kiahna-tucker kiahna-tucker requested a review from psFried January 14, 2025 16:02
{intl.formatMessage({ id: headerMessageId })}
</Typography>

{Hydrating ?? children}
Copy link
Member

Choose a reason for hiding this comment

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

Whenever there is a capitalized props like Hydrating I assume it takes in a functional component and used like

<Hydrating {...whatever props you might need}/>

In this scenario it looks like all 3 uses of this are passing in a skeleton with different sizes. I think we should just pass in the sizing they want and then this component itself can handle checking hydration.

Copy link
Member

Choose a reason for hiding this comment

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

Mentioned in dailies... but putting here so there is an actual record of this.

I think we should move this out of deps as that is mainly for dependencies that are getting pulled in from external places.

accessToken: string,
catalogName: string
): Promise<EntityStatusResponse[]> =>
client(`${entityStatusBaseEndpoint}?name=${catalogName}`, {}, accessToken);
Copy link
Member

Choose a reason for hiding this comment

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

I think probably best to do it here - but we need to make sure catalogName is safe / escaped. Normally we have used escapeReservedCharacters but that is really for PostgREST so not 100% sure what we wanna do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:planned This is a planned change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants