Skip to content

Conversation

@TuTiDore
Copy link
Contributor

@TuTiDore TuTiDore commented Dec 17, 2025

resolves #144

/shockers/logs/
screenshot-2025-12-20_15-53-05

/shockers/logs/{id}
screenshot-2025-12-20_15-53-23

@changeset-bot
Copy link

changeset-bot bot commented Dec 17, 2025

⚠️ No Changeset found

Latest commit: 1080559

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a shocker logs page accessible via the route /shockers/logs/{shockerId}, allowing users to view historical log entries for individual shockers. The implementation includes a new store for managing log data, a logs page with a sortable data table, and a menu button on shocker control cards to navigate to the logs.

Key changes:

  • New ShockerLogStore to fetch and cache shocker logs
  • New logs page component with table displaying time, type, controller, intensity, and duration
  • Added dropdown menu to shocker control cards with "View Logs" option

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
src/lib/stores/ShockerLogStore.ts New store for fetching and caching shocker logs with Map-based storage
src/routes/(authenticated)/shockers/logs/[shockerId=guid]/+page.svelte New page displaying logs in a sortable table with refresh functionality
src/lib/components/ControlModules/impl/ShockerMenu.svelte New dropdown menu component for shocker actions
src/lib/components/ControlModules/ClassicControlModule.svelte Integrated ShockerMenu into shocker card title

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@TuTiDore
Copy link
Contributor Author

My broad questions are

  1. Is hamburger/ dropdown ok, how should it look for other controllers, especially simple?
  2. Would you typically create a store for a collection of endpoints, or do you ever not need a store. Is having a map on shockerId typical/ fine?
  3. Should it try to use all V2 API, or should new endpoints be added if needed?

@LucHeart
Copy link
Member

  1. thats fine yeah
  2. depends, i think here its fine to just have it in the component aswell, but as soon as it gets more complicated a store / svelte.ts file with states makes things a lot easier
  3. If there is one, always prefer, if you feel like you need to do multiple requests per n, then a new endpoint is in order yeah

In general I more so wanted to make a general log page, that just has all logs combined, instead of per shocker. If you wanna adpot that idea, feel free, if not then thats just gonna be at a later point :D

Thank you for contributing :3

fix: wording

Co-authored-by: LucHeart <luc@luc.cat>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@TuTiDore TuTiDore force-pushed the feat/shocker-logs-page branch from a7f178b to b507c13 Compare December 20, 2025 19:59
@TuTiDore
Copy link
Contributor Author

force pushed to sign my commit. ready for re-review @LucHeart

@hhvrc
Copy link
Contributor

hhvrc commented Dec 21, 2025

This looks great, will approve once this gets updated to use the new endpoint 😄

@hhvrc hhvrc added the enhancement New feature or request label Dec 21, 2025
@hhvrc hhvrc added this to Roadmap Dec 21, 2025
@github-project-automation github-project-automation bot moved this to Todo in Roadmap Dec 21, 2025
@hhvrc hhvrc moved this from Todo to In Progress in Roadmap Dec 21, 2025
@hhvrc hhvrc modified the milestone: Frontend 1.0 MVP Release Dec 21, 2025
@TuTiDore
Copy link
Contributor Author

TuTiDore commented Dec 22, 2025

I used pnpx @openapitools/openapi-generator-cli generate and it wants to remove the .openapi-generator-ignore in FILES but edd0c4b added them. Do you know why this is the case, should they be there?

@hhvrc
Copy link
Contributor

hhvrc commented Dec 22, 2025

I used pnpx @openapitools/openapi-generator-cli generate and it wants to remove the .openapi-generator-ignore in FILES but edd0c4b added them. Do you know why this is the case, should they be there?

@TuTiDore this seems to be something that openapi generator wants to add and remove all the time i dont know why, ive defaulted to just ignoring when openapi generator wants to remove it and just never commit it 😄

Copy link
Contributor

@hhvrc hhvrc left a comment

Choose a reason for hiding this comment

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

Nice work!
LGTM 😄

@hhvrc hhvrc merged commit 500f7c3 into OpenShock:develop Dec 22, 2025
4 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Roadmap Dec 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[MVP] Authenticated User Shocker Logs page

3 participants