Skip to content

Conversation

Tristan-WorkGH
Copy link
Collaborator

@Tristan-WorkGH Tristan-WorkGH commented Jan 26, 2024

GridAdmin need to use multiples servers while being authenticate, so mock auth isn't usable as is.

Also split rest API in modules for better readability.

Copy link
Contributor

@TheMaskedTurtle TheMaskedTurtle left a comment

Choose a reason for hiding this comment

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

All good for the authentication part :)

.env Outdated
Comment on lines 1 to 9
EXTEND_ESLINT=true

REACT_APP_API_GATEWAY=api/gateway
REACT_APP_WS_GATEWAY=ws/gateway
REACT_APP_API_GATEWAY=/api/gateway
REACT_APP_WS_GATEWAY=/ws/gateway

EXTEND_ESLINT=true
REACT_APP_SRV_STUDY_URI=study
REACT_APP_SRV_CONFIG_URI=config
REACT_APP_SRV_CONFIG_NOTIF_URI=config-notification
REACT_APP_SRV_USER_ADMIN_URI=user-admin

Choose a reason for hiding this comment

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

it's a bit confusing to have something that different from gridstudy-app

  • REACT_APP_API_GATEWAY=/api/gateway : there is not any "/" in front of the URL in gridstudy
  • server URI are defined in service files directly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • it's a fix : we define all URLs in services like STUDY_URL = ``${process.env.REACT_APP_API_GATEWAY}/${process.env.REACT_APP_SRV_STUDY_URI}/v1``;, so we already have a slash in url, no use to have double
    it don't pose a problem in gridstudy for example because consecutive slashes are treated like one only for URL
  • it's to offer more configuration of the application without having to edit all files
    we can configure the gateway path, so why not also the service name/path ?

@Tristan-WorkGH Tristan-WorkGH merged commit 7541a1d into main Feb 5, 2024
@Tristan-WorkGH Tristan-WorkGH deleted the remove_mock_auth branch February 5, 2024 11:18
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