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.

Sign up for GitHub

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

Jump to bottom

Activity Log backend and WebUI #110

Merged
merged 21 commits into from
Sep 15, 2022
Merged

Activity Log backend and WebUI #110

merged 21 commits into from
Sep 15, 2022

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Sep 8, 2022 •
edited
Loading

  • feat: add ActivityLog backend
  • feat: record ActivityLog entries

See #81

juliangruber
juliangruber reviewed Sep 8, 2022
View reviewed changes
main/activity-log.js Outdated Show resolved Hide resolved
main/activity-log.js Outdated Show resolved Hide resolved
main/activity-log.js Outdated Show resolved Hide resolved
main/activity-log.js Outdated Show resolved Hide resolved
juliangruber
juliangruber reviewed Sep 8, 2022
View reviewed changes
main/activity-log.js Outdated Show resolved Hide resolved
main/activity-log.js Outdated Show resolved Hide resolved
main/index.js Show resolved Hide resolved
main/saturn-node.js Show resolved Hide resolved
bajtos
bajtos commented Sep 8, 2022
View reviewed changes
main/typings.d.ts Outdated Show resolved Hide resolved
bajtos
bajtos commented Sep 8, 2022
View reviewed changes
renderer/src/main.tsx Outdated Show resolved Hide resolved
bajtos
bajtos commented Sep 8, 2022
View reviewed changes
main/activity-log.js Outdated Show resolved Hide resolved
@bajtos bajtos marked this pull request as ready for review September 8, 2022 14:00
@bajtos
Copy link
Member Author

bajtos commented Sep 8, 2022

The PR is ready for final review.

@bajtos
Copy link
Member Author

bajtos commented Sep 8, 2022

@juliangruber Do you have any opinions on testing the renderer integration?

Ideally, we should have end-to-end tests to verify that window.electron.getActivityLog() and ndow.electron.onActivityLogged work correctly. On the other hand, writing and maintaining e2e tests comes with extra overhead, I am not sure if it's a good ROI at this stage of our project.

@bajtos
Copy link
Member Author

bajtos commented Sep 8, 2022

@juliangruber Do you have any opinions on testing the renderer integration?

Ideally, we should have end-to-end tests to verify that window.electron.getActivityLog() and ndow.electron.onActivityLogged work correctly. On the other hand, writing and maintaining e2e tests comes with extra overhead, I am not sure if it's a good ROI at this stage of our project.

To keep iterating fast, if we decide to add e2e tests, then I am proposing to do so in a follow-up pull request.

@juliangruber
Copy link
Member

@juliangruber Do you have any opinions on testing the renderer integration?

Ideally, we should have end-to-end tests to verify that window.electron.getActivityLog() and ndow.electron.onActivityLogged work correctly. On the other hand, writing and maintaining e2e tests comes with extra overhead, I am not sure if it's a good ROI at this stage of our project.

From my perspective we don't need tests for this at this stage. If anything we could start with IPC level tests, before looking at testing DOM results. And +1 to implementing both later.

juliangruber
juliangruber requested changes Sep 9, 2022
View reviewed changes
main/activity-log.js Outdated Show resolved Hide resolved
main/activity-log.js Outdated Show resolved Hide resolved
main/index.js Show resolved Hide resolved
main/saturn-node.js Show resolved Hide resolved
main/typings.d.ts Outdated Show resolved Hide resolved
main/index.js Outdated Show resolved Hide resolved
main/ui.js Outdated Show resolved Hide resolved
@bajtos
feat: add ActivityLog backend
45d9eb0 
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
feat: record ActivityLog entries
733f4e2 
- When the Station starts/stops
- When the Saturn Module starts/stops
- INFO & ERROR logs from Saturn

Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
feat: expose ActivityLog to renderer
ac0abd9 
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
fixup! address Julian comments
aad1f6e 
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
fixup! use String(val) instead of '' + val
780b762 
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
fixup! remove unused clone()
610e8cf 
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
fixup! import types via JSDoc
3e97d6a 
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
feat: limit ActivityLog size to 100 items
4b8ddab 
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
fixup! remove forgotten console log
0359412 
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
fixup! add more typedefs
4fd20c4 
Signed-off-by: Miroslav Bajtoš <[email protected]>
bajtos
bajtos commented Sep 13, 2022
View reviewed changes
renderer/src/main.tsx Outdated Show resolved Hide resolved
juliangruber
juliangruber requested changes Sep 13, 2022
View reviewed changes
Copy link
Member

@juliangruber juliangruber left a comment

Choose a reason for hiding this comment

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

see review comments above

@juliangruber juliangruber mentioned this pull request Sep 14, 2022
Merged
juliangruber and others added 3 commits September 15, 2022 13:18
@juliangruber
Refactor activity log types (#127)
d8e010f 
* prototype activity log layout

* fix types

* refactor types

* refactor names

* refactor variable names

* fix sorting bug

* docs
@bajtos
fix: fix eslint issue in tests
cf3e3f1 
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos @juliangruber
refactor: with Julian
cde1d46 
Co-authored-by: Julian Gruber <[email protected]>
Signed-off-by: Miroslav Bajtoš <[email protected]>
@bajtos
Copy link
Member Author

bajtos commented Sep 15, 2022 •
edited
Loading

We did a pair programming session with @juliangruber today and brought the PR to the finish line.

@ronaldofs @jopedroliveira We implemented a simple ActivityLog component that's consuming backend API and rendering a list of events. See renderer/src/components/ActivityLog.tsx and renderer/src/App.tsx. I hope this will make it super easy for you to integrate our changes.

Feel free to rework both App.tsx and ActivityLog.tsx in any way you like. Our code is just a minimal working example to demonstrate that the backend integration is working.

@bajtos bajtos enabled auto-merge (squash) September 15, 2022 13:25
@bajtos bajtos merged commit 7fca41b into main Sep 15, 2022
@bajtos bajtos deleted the feat-activity-log-backend branch September 15, 2022 13:59
@bajtos bajtos changed the title Activity Log backend Activity Log backend and WebUI Sep 15, 2022
@bajtos bajtos mentioned this pull request Sep 15, 2022
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliangruber juliangruber juliangruber approved these changes

@ronaldofs ronaldofs Awaiting requested review from ronaldofs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bajtos @juliangruber @ronaldofs