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

Refactor frontend, relates to issue #35 #43

Closed
wants to merge 47 commits into from

Conversation

LaumiH
Copy link

@LaumiH LaumiH commented Jun 29, 2023

Hi all,

with this PR I want to share my refactored webconsole frontend. I made small changes and fixes in the backend as well.

This is a non-exhaustive list of the improvements and changes I made:

  • update react dependencies and used libraries
  • update react components, do not extend React.Component but are functions for easier lifecycle management
  • remove webpack; use react-scripts
  • improve pages folder structure
  • improve page navigation and history
  • improve API calls: tightly couple redux state and http calls
  • add possibility to fake backend calls for testing and development
  • improve usability of subscriber page/ modal using react hooks form, add more validation and improved feedback for users
  • add duplicate subscriber functionality
  • add validation for duplicate imsi
  • consolidate and improve subscriber marshalling (frontend form <-> backend)
  • add configurable auto-refresh functionality
  • add golang panic handling to backend and display errors better in frontend
  • fix reload issue; reloading page does not lead to login page
  • improve configurability of server IP and frontend/ backend port
  • add developer hints to readme to explain hot-reload feature of react

I plan to subsequently add my newly developed functionality for the webconsole to this PR.

Please let me know what you think, add comments and requests for changes!

@ianchen0119
Copy link
Contributor

Hi @LaumiH

Thanks for your contribution,
I will review all of changes.

@ianchen0119
Copy link
Contributor

Hi @ericboy0224

Please help to review this PR when you feel free.
Thanks.

return
}
//msisdnTemp += 1
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix:

gofumpt -w backend/WebUI/api_webui.go

// sendResponseToClient(c, resp)
// } else {
// sendResponseToClientFilterTenant(c, resp, tenantId)
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to remove unused part?

Copy link
Author

Choose a reason for hiding this comment

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

I leave that to you, I don't want to refactor too much in the backend.

backend/WebUI/routers.go Show resolved Hide resolved
.gitignore Show resolved Hide resolved

<div class="clearfix"></div>
<script src="https://cdn.jsdelivr.net/npm/react/umd/react.production.min.js" crossorigin></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest that do not use CDN resources in the webconsole.
Because in some use cases, our user could deploy the free5GC in their private network without internet.

* fix: CSRF vulnerability

* Use jwtKey (256 bytes random string) as JWT Signature Private Key

* Delete add Add admin's tenant & user when initializing Backend

fix linter

* fix: exposed password in UserModal

* add confirmPassword in UserModal

* use crypto/rand instead of math/rand

* Use http.StatusUnauthorized instead of http.StatusNotFound when CheckAuth failed.

* add InitJwtKey()
@ianchen0119
Copy link
Contributor

ianchen0119 commented Jul 21, 2023

Hi @LaumiH
Please help to resolve the conflicts.
Moreover,
Would you please provide the time schedule of this PR?
The reason of this request is due to we have other development plans, so we are considering how to merge all of changes.

Thank you.

@brianchennn
Copy link
Contributor

@LaumiH
I am sorry but have to tell you that our team prefer the PR #47. We may merge it after the bugs are fixed.

@LaumiH
Copy link
Author

LaumiH commented Aug 3, 2023

Sad to hear that!

Should you ever want to reconsider this decision, you know where to find my repo :) Come check out the new functionality, too.

Cheers,
LaumiH

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.

3 participants