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

Show version info on webui #205

Merged
merged 12 commits into from
Mar 9, 2024
Merged

Show version info on webui #205

merged 12 commits into from
Mar 9, 2024

Conversation

poruta99
Copy link
Contributor

@poruta99 poruta99 commented Mar 8, 2024

See: #81

Fetch version info from /status endpoint and show it right after the icon on the menu bar.

image

Added status module and version component. The version component simply
fetch the version info from the /status endpoint on page init.
@poruta99
Copy link
Contributor Author

poruta99 commented Mar 8, 2024

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

image

@mgdigital
Copy link
Collaborator

mgdigital commented Mar 8, 2024

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!)

@poruta99
Copy link
Contributor Author

poruta99 commented Mar 8, 2024

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.

@mgdigital
Copy link
Collaborator

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...

@mgdigital
Copy link
Collaborator

Quick guide to implementing the GraphQL:

  1. Add the query in graphql/schemas (maybe under Query -> system -> version (string))
  2. Generate the boilerplate resolver with go run github.com/99designs/gqlgen generate --config ./internal/gql/gqlgen.yml
  3. Implement the resolver in internal/gql/resolvers (it needs to import the version from internal/version)
  4. Add a query in graphql/queries
  5. Generate the frontend GraphQL code with npm graphql:codegen
  6. Integrate the frontend component

@poruta99
Copy link
Contributor Author

poruta99 commented Mar 8, 2024

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:

image

@poruta99
Copy link
Contributor Author

poruta99 commented Mar 8, 2024

Quick guide to implementing the GraphQL:

  1. Add the query in graphql/schemas (maybe under Query -> system -> version (string))
  2. Generate the boilerplate resolver with go run github.com/99designs/gqlgen generate --config ./internal/gql/gqlgen.yml
  3. Implement the resolver in internal/gql/resolvers (it needs to import the version from internal/version)
  4. Add a query in graphql/queries
  5. Generate the frontend GraphQL code with npm graphql:codegen
  6. Integrate the frontend component

Thank you for the detailed guide, should I finish the status GraphQL API refactoring in this PR?

@mgdigital
Copy link
Collaborator

Quick guide to implementing the GraphQL:

  1. Add the query in graphql/schemas (maybe under Query -> system -> version (string))
  2. Generate the boilerplate resolver with go run github.com/99designs/gqlgen generate --config ./internal/gql/gqlgen.yml
  3. Implement the resolver in internal/gql/resolvers (it needs to import the version from internal/version)
  4. Add a query in graphql/queries
  5. Generate the frontend GraphQL code with npm graphql:codegen
  6. 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.

@mgdigital
Copy link
Collaborator

I didn't use GraphQL before

If you're stuck on implementing this I can update the PR when I get to it.

@poruta99
Copy link
Contributor Author

poruta99 commented Mar 8, 2024

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.

image

version
}
}
}
Copy link
Collaborator

@mgdigital mgdigital Mar 9, 2024

Choose a reason for hiding this comment

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

I think this should be:

query SystemQuery {
  system {
      version
  }
}

@mgdigital
Copy link
Collaborator

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.

import * as generated from "../../graphql/generated";
import { GraphQLService } from 'src/app/graphql/graphql.service';

const defaultVersionName = 'N/A';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think maybe default to an empty string as you might get a flash of N/A on initial load with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will fix this later.

@poruta99
Copy link
Contributor Author

poruta99 commented Mar 9, 2024

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

@poruta99
Copy link
Contributor Author

poruta99 commented Mar 9, 2024

Do I need to rebase this branch to keep commits less and clean?

@@ -96,6 +96,15 @@ export class GraphQLService {
})
.pipe(map((r) => r.data.torrent.suggestTags));
}

systemStatusQeury(): Observable<generated.SystemQuery> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably just be systemQuery

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@mgdigital
Copy link
Collaborator

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.

@poruta99
Copy link
Contributor Author

poruta99 commented Mar 9, 2024

Also your test is still failing on here:

That's strange, I can serve or build locally without this error.

@mgdigital
Copy link
Collaborator

mgdigital commented Mar 9, 2024

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();

@mgdigital
Copy link
Collaborator

I assume ng test must still be failing locally for you as it can't currently resolve anything for the GraphQL service?

@poruta99
Copy link
Contributor Author

poruta99 commented Mar 9, 2024

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...

@poruta99
Copy link
Contributor Author

poruta99 commented Mar 9, 2024

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.

image

@mgdigital mgdigital merged commit f6edd02 into bitmagnet-io:main Mar 9, 2024
9 checks passed
@mgdigital
Copy link
Collaborator

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....

@poruta99
Copy link
Contributor Author

poruta99 commented Mar 9, 2024

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.

truecharts-admin added a commit to truecharts/charts that referenced this pull request Mar 11, 2024
… v0.7.12@07c9c43 by renovate (#19063)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
|
[ghcr.io/bitmagnet-io/bitmagnet](https://togithub.com/bitmagnet-io/bitmagnet)
| patch | `0.7.7` -> `0.7.12` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>bitmagnet-io/bitmagnet
(ghcr.io/bitmagnet-io/bitmagnet)</summary>

###
[`v0.7.12`](https://togithub.com/bitmagnet-io/bitmagnet/releases/tag/v0.7.12)

[Compare
Source](https://togithub.com/bitmagnet-io/bitmagnet/compare/v0.7.10...v0.7.12)

#### What's Changed

- Fix Torznab content type criteria by
[@&#8203;mgdigital](https://togithub.com/mgdigital) in
[bitmagnet-io/bitmagnet#209
- fix duplicate word in index.md by
[@&#8203;sweetbbak](https://togithub.com/sweetbbak) in
[bitmagnet-io/bitmagnet#208
- Add external resources page by
[@&#8203;mgdigital](https://togithub.com/mgdigital) in
[bitmagnet-io/bitmagnet#202
- Upgrade GitHub Actions and fix Docker build by
[@&#8203;mgdigital](https://togithub.com/mgdigital) in
[bitmagnet-io/bitmagnet#201
- Fix actions: Use name and key for matrix by
[@&#8203;mgdigital](https://togithub.com/mgdigital) in
[bitmagnet-io/bitmagnet#210

#### New Contributors

- [@&#8203;sweetbbak](https://togithub.com/sweetbbak) made their first
contribution in
[bitmagnet-io/bitmagnet#208

**Full Changelog**:
bitmagnet-io/bitmagnet@v0.7.10...v0.7.12

###
[`v0.7.10`](https://togithub.com/bitmagnet-io/bitmagnet/releases/tag/v0.7.10)

[Compare
Source](https://togithub.com/bitmagnet-io/bitmagnet/compare/v0.7.9...v0.7.10)

#### What's Changed

- Show version info on webui by
[@&#8203;poruta99](https://togithub.com/poruta99) in
[bitmagnet-io/bitmagnet#205
- Fix metrics endpoint by
[@&#8203;mgdigital](https://togithub.com/mgdigital) in
[bitmagnet-io/bitmagnet#207

#### New Contributors

- [@&#8203;poruta99](https://togithub.com/poruta99) made their first
contribution in
[bitmagnet-io/bitmagnet#205

**Full Changelog**:
bitmagnet-io/bitmagnet@v0.7.9...v0.7.10

###
[`v0.7.9`](https://togithub.com/bitmagnet-io/bitmagnet/releases/tag/v0.7.9)

[Compare
Source](https://togithub.com/bitmagnet-io/bitmagnet/compare/v0.7.8...v0.7.9)

#### What's Changed

- Fix Docker build by
[@&#8203;mgdigital](https://togithub.com/mgdigital) in
[bitmagnet-io/bitmagnet#200

**Full Changelog**:
bitmagnet-io/bitmagnet@v0.7.8...v0.7.9

###
[`v0.7.8`](https://togithub.com/bitmagnet-io/bitmagnet/releases/tag/v0.7.8)

[Compare
Source](https://togithub.com/bitmagnet-io/bitmagnet/compare/v0.7.7...v0.7.8)

This release includes several minor bug fixes.

#### What's Changed

- Fix comma-separated Torznab categories by
[@&#8203;mgdigital](https://togithub.com/mgdigital) in
[bitmagnet-io/bitmagnet#194
- Rebuild webui by [@&#8203;mgdigital](https://togithub.com/mgdigital)
in
[bitmagnet-io/bitmagnet#195
- Fix Postgres call on CLI by
[@&#8203;mgdigital](https://togithub.com/mgdigital) in
[bitmagnet-io/bitmagnet#196
- Upgrade actions by [@&#8203;mgdigital](https://togithub.com/mgdigital)
in
[bitmagnet-io/bitmagnet#198

#### New Contributors

- [@&#8203;RealFascinated](https://togithub.com/RealFascinated) made
their first contribution in
[bitmagnet-io/bitmagnet#193
- [@&#8203;Ornias1993](https://togithub.com/Ornias1993) made their first
contribution in
[bitmagnet-io/bitmagnet#188

**Full Changelog**:
bitmagnet-io/bitmagnet@v0.7.7...v0.7.8

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 10pm on monday" in timezone
Europe/Amsterdam, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMzUuMSIsInVwZGF0ZWRJblZlciI6IjM3LjIzNS4xIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIn0=-->
@poruta99 poruta99 deleted the webui-menu-bar-version branch March 11, 2024 02:06
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.

None yet

2 participants