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

feat: display external portals on readme page + test coverage #4177

Merged
merged 46 commits into from
Jan 29, 2025

Conversation

jamdelion
Copy link
Contributor

@jamdelion jamdelion commented Jan 21, 2025

In this PR:

  • Display external portals list on Read Me Page (including refactoring component so usable across multiple places)
  • Add storybook file for ReadMePage
  • Add test coverage for ReadMePage (one issue encountered, see below ⬇️ )

Testing issues

In the test it("displays data in the fields if there is already flow information in the database", it only worked if I set just one piece of state. If I set two fields in state:

await act(async () =>
      setState({
        flowSummary: "This flow summary is in the db already",
        flowDescription: "Something else",
      })
    );

...it would find the first piece of text in the document but not the second. When I logged what was in the store on the component page, both pieces of text were in the store on a first render, and then both were "" on the second render. With screen.debug only serviceSummary had a value displayed.

I'm wondering if this is a timing issue and too many re-renders, but wasn't really sure how to debug where the re-renders are coming from. I wasn't able to recreate this with the profiling tool in React dev tools (although it did show a high number of re-renders when the inputs are typed in, which makes sense as they re-render on every keystroke).

Copy link

github-actions bot commented Jan 21, 2025

🤖 Hasura Change Summary compared a subset of table metadata including permissions:

Updated Tables (2)

  • public.flows permissions:

    insert select update delete
    api / / /
    demoUser / / /
    platformAdmin / / /
    teamEditor / / /
    24 added column permissions
    insert select update
    api ➕ limitations
    ➕ summary
    ➕ limitations
    ➕ summary
    ➕ limitations
    ➕ summary
    demoUser
    platformAdmin
    teamEditor
  • public.published_flows

@jamdelion jamdelion changed the title jh/read me follow ons feat: display more info on readme page Jan 21, 2025
@jamdelion jamdelion changed the base branch from main to jh/experimental-readme-page January 21, 2025 22:12
Copy link

github-actions bot commented Jan 21, 2025

Removed vultr server and associated DNS entries

@jamdelion jamdelion changed the title feat: display more info on readme page feat: display external portals on readme page + test coverage Jan 23, 2025
@jamdelion jamdelion mentioned this pull request Jan 23, 2025
3 tasks
@jamdelion jamdelion marked this pull request as ready for review January 23, 2025 21:17
Base automatically changed from jh/experimental-readme-page to main January 28, 2025 18:40
Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

Looking great - a few very minor comments.

Couldn't get to the bottom of the testing issue, but totally agree it's non-functional and something to do with the interacting of vitest and zustand - all working as expected for a user 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that this is shared outside the context of search, we should probably move it to a shared location.

@jamdelion jamdelion merged commit f239666 into main Jan 29, 2025
13 checks passed
@jamdelion jamdelion deleted the jh/read-me-follow-ons branch January 29, 2025 13:05
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.

2 participants