Closed poruta99 closed 2 months ago
The webui is still not built and commited, I'm not sure the build instruction in the package.json is correct. After I build the project, there some untracked files generated, and the filenames looks different. Would you mind have a look on it? Thanks. @mgdigital
Hey thanks for this!
I hadn't thought of making the version so prominent in the heading but I kinda like it... I was thinking to make something like a status drawer to include the version and a health check indicator, but that needs a bit more work.
One thing - I think this needs to come from the GraphQL API as we don't wanna be talking to 2 separate APIs. So we'd need to add a GraphQL query for it. Is this something you're able to look at?
To build the embedded web UI you'd need to run ng build -c embedded
- you can find various useful commands for local development here: https://github.com/bitmagnet-io/bitmagnet/blob/main/Taskfile.yml (yes the development docs could probably be improved!)
Hey thanks for this!
I hadn't thought of making the version so prominent in the heading but I kinda like it... I was thinking to make something like a status drawer to include the version and a health check indicator, but that needs a bit more work.
One thing - I think this needs to come from the GraphQL API as we don't wanna be talking to 2 separate APIs. So we'd need to add a GraphQL query for it. Is this something you're able to look at?
To build the embedded web UI you'd need to run
ng build -c embedded
- you can find various useful commands for local development here: https://github.com/bitmagnet-io/bitmagnet/blob/main/Taskfile.yml (yes the development docs could probably be improved!)
Sure. Using 2 kinds of APIs do look kind of weird.
I would like to see if I can implement it in GraphQL and then implement the status drawer.
I'm thinking we can keep the version where you've put it, could we make the font size a few points smaller than the bitmagnet
though?
Maybe the health indicator/drawer should be separate PR? I haven't really thought about how it needs to look, if it needs to be a drawer or maybe just like a ⚠️ icon when something is wrong...
Quick guide to implementing the GraphQL:
graphql/schemas
(maybe under Query -> system -> version (string))go run github.com/99designs/gqlgen generate --config ./internal/gql/gqlgen.yml
internal/gql/resolvers
(it needs to import the version from internal/version
)graphql/queries
npm graphql:codegen
Maybe the health indicator/drawer should be separate PR?
Sure.
What about the GraphQL refactoring, also comes with another PR? I didn't use GraphQL before, so I made this workaround version. : )
The font size is updated:
Quick guide to implementing the GraphQL:
- Add the query in
graphql/schemas
(maybe under Query -> system -> version (string))- Generate the boilerplate resolver with
go run github.com/99designs/gqlgen generate --config ./internal/gql/gqlgen.yml
- Implement the resolver in
internal/gql/resolvers
(it needs to import the version frominternal/version
)- Add a query in
graphql/queries
- Generate the frontend GraphQL code with
npm graphql:codegen
- Integrate the frontend component
Thank you for the detailed guide, should I finish the status GraphQL API refactoring in this PR?
Quick guide to implementing the GraphQL:
- Add the query in
graphql/schemas
(maybe under Query -> system -> version (string))- Generate the boilerplate resolver with
go run github.com/99designs/gqlgen generate --config ./internal/gql/gqlgen.yml
- Implement the resolver in
internal/gql/resolvers
(it needs to import the version frominternal/version
)- Add a query in
graphql/queries
- Generate the frontend GraphQL code with
npm graphql:codegen
- Integrate the frontend component
Thank you for the detailed guide, should I finish the status GraphQL API refactoring in this PR?
Yeah I think the version would have to come from GraphQL in this PR, as otherwise we're adding a dependency on the status endpoint.
I didn't use GraphQL before
If you're stuck on implementing this I can update the PR when I get to it.
I reworked the code, now it's using GraphQL API to fetch system status. I'm not sure the GraphQL query schema is well-defined.
Thanks, I think it's nearly there, have added a few comments.
That dirty
in the version number is expected until we push a new version tag, it just means you have changes in your build since the last version.
I've run the CI checks that have highlighted some issues, you need to run prettier
on your code and your test is failing.
I've pushed new commits on it. I believe the issues are fixed right now, please have a look again.
Thank you for your patient review. @mgdigital
Do I need to rebase this branch to keep commits less and clean?
Looks like it's nearly there. You need to run npx prettier --write .
from the root of the project to pick up any formatting issues. Also your test is still failing on here:
Unexpected directive 'VersionComponent' imported by the module 'DynamicTestModule'. Please add an @NgModule annotation.
Do I need to rebase this branch to keep commits less and clean?
No that's fine, the PR will be squash merged.
Also your test is still failing on here:
That's strange, I can serve or build locally without this error.
Also your test is still failing on here:
That's strange, I can serve or build locally without this error.
Should the component here be under declarations
rather than imports
? (or the import should be the StatusModule
?)
await TestBed.configureTestingModule({
imports: [VersionComponent],
}).compileComponents();
I assume ng test
must still be failing locally for you as it can't currently resolve anything for the GraphQL service?
I assume
ng test
must still be failing locally for you as it can't currently resolve anything for the GraphQL service?
Sorry, I run ng test
in wsl, it prompts me that I don't have a Chrome bin available. I'm trying to fix this...
I assume
ng test
must still be failing locally for you as it can't currently resolve anything for the GraphQL service?Sorry, I run
ng test
in wsl, it prompts me that I don't have a Chrome bin available. I'm trying to fix this...
ng test
pass locally.
Thanks for this! I can suggest more stuff if you're interested in working on the web UI. One of the next things is getting Angular Routing working so we can have more than just a single page....
Thanks for this! I can suggest more stuff if you're interested in working on the web UI. One of the next things is getting Angular Routing working so we can have more than just a single page....
Maybe not, I am not an expert in Angular, anyway thanks for the merge and help.
See: #81
Fetch version info from /status endpoint and show it right after the icon on the menu bar.