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

v9: Created new system information section in help panel #11230

Merged
merged 30 commits into from
Oct 7, 2021

Conversation

Zeegaan
Copy link
Member

@Zeegaan Zeegaan commented Oct 1, 2021

image

Notes

  • Created a new System Information section, within the "help" section
  • Created a new UserData Model
  • Created IUserDataService Interface
  • Created UserDataService class and registered the service
  • Created Unit Tests to test the service
  • Created Cypress test to System Information displayed

How to test

  • Navigate to the client project and run npm install & npm run build
  • Run the project
  • Open the help section and the System Information and see that all values display correctly
  • Try updating your .NET 5 to a newer version and see if it registers the change
  • Try changing your users language and see if it registers the change(You have to refresh the page after changes)
  • Try going to the backoffice from a different browser
  • Try deleting your browser of choice from platform.service.js (either with npm run dev already running, or rebuilding the client project with npm install & npm run build

Copy link
Contributor

@nikolajlauridsen nikolajlauridsen left a comment

Choose a reason for hiding this comment

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

Found a couple of things in the code, I haven't tested anything yet though 😄

Nikolaj Geisle and others added 5 commits October 4, 2021 11:29
Signed-off-by: Nikolaj Geisle <[email protected]>
Signed-off-by: Nikolaj Geisle <[email protected]>
Signed-off-by: Nikolaj Geisle <[email protected]>
Signed-off-by: Nikolaj Geisle <[email protected]>
Copy link
Contributor

@nikolajlauridsen nikolajlauridsen left a comment

Choose a reason for hiding this comment

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

Looks good now, and all tests are passing 🎉, let's get this merged

@abjerner
Copy link
Contributor

abjerner commented Oct 7, 2021

Looks great 👍

Since there is already some information about the server, wouldn't it make sense to also include the machine name?

I know there should ever only be a single master server in a load balanced setup, but having the machine there would help confirm that the user is hitting the right server.

@Zeegaan
Copy link
Member Author

Zeegaan commented Oct 7, 2021

Hi @abjerner and thanks for the suggestion 👍
This system information display is primarily for posting along an issue report, so we can see what state the server is in at the moment of the issue, so I unfortunately don't think that machine name is relevant for us, and if you'd post it in the issue, you could possibly be disclosing sensitive information aswell.

You're free to add this to the user Dashboard though, here's a PR that allows for custom dashboards:
#10576

Thanks again for the comment! 🚀

@Zeegaan Zeegaan merged commit d6b4fd5 into v9/dev Oct 7, 2021
@Zeegaan Zeegaan deleted the v9/feature/debug-dashboard branch October 7, 2021 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants