-
Notifications
You must be signed in to change notification settings - Fork 16
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
chore: updates for login_app type [LIBS-405] #788
Changes from 2 commits
2be3f4e
f6abdd0
0df2188
151357d
fdcd8a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ export const ServerVersionProvider = ({ | |
url, | ||
apiVersion, | ||
pwaEnabled, | ||
loginApp, | ||
children, | ||
}) => { | ||
const offlineInterface = useOfflineInterface() | ||
|
@@ -27,28 +28,39 @@ export const ServerVersionProvider = ({ | |
} | ||
|
||
setState((state) => (state.loading ? state : { loading: true })) | ||
const request = get(`${url}/api/system/info`) | ||
request | ||
.then((systemInfo) => { | ||
setState({ loading: false, systemInfo }) | ||
}) | ||
.catch((e) => { | ||
// Todo: If this is a network error, the app cannot load -- handle that gracefully here | ||
// if (e === 'Network error') { ... } | ||
setState({ loading: false, error: e }) | ||
}) | ||
|
||
return () => { | ||
request.abort() | ||
// in reality we probably want a request to get api version | ||
if (loginApp) { | ||
const fakeSystemInfo = { version: '2.40-SNAPSHOT' } | ||
setState({ loading: false, systemInfo: fakeSystemInfo }) | ||
Comment on lines
+33
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated |
||
} else { | ||
const request = get(`${url}/api/system/info`) | ||
request | ||
.then((systemInfo) => { | ||
setState({ loading: false, systemInfo }) | ||
}) | ||
.catch((e) => { | ||
// Todo: If this is a network error, the app cannot load -- handle that gracefully here | ||
// if (e === 'Network error') { ... } | ||
setState({ loading: false, error: e }) | ||
}) | ||
|
||
return () => { | ||
request.abort() | ||
} | ||
} | ||
}, [url]) | ||
}, [url, loginApp]) | ||
|
||
if (loading) { | ||
return <LoadingMask /> | ||
} | ||
|
||
if (error) { | ||
return <LoginModal /> | ||
return !loginApp ? ( | ||
<LoginModal /> | ||
) : ( | ||
<p>Specify DHIS2_BASE_URL environment variable</p> | ||
) | ||
} | ||
|
||
const serverVersion = parseDHIS2ServerVersion(systemInfo.version) | ||
|
@@ -65,7 +77,8 @@ export const ServerVersionProvider = ({ | |
systemInfo, | ||
pwaEnabled, | ||
}} | ||
offlineInterface={offlineInterface} | ||
offlineInterface={loginApp ? null : offlineInterface} | ||
loginApp={loginApp} | ||
> | ||
{children} | ||
</Provider> | ||
|
@@ -77,6 +90,7 @@ ServerVersionProvider.propTypes = { | |
appVersion: PropTypes.string.isRequired, | ||
apiVersion: PropTypes.number, | ||
children: PropTypes.element, | ||
loginApp: PropTypes.bool, | ||
pwaEnabled: PropTypes.bool, | ||
url: PropTypes.string, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ const { reporter, prettyPrint } = require('@dhis2/cli-helpers-engine') | |
const chokidar = require('chokidar') | ||
const fs = require('fs-extra') | ||
const makeBabelConfig = require('../../../config/makeBabelConfig.js') | ||
const { appTypes } = require('../parseConfig') | ||
const { | ||
verifyEntrypoints, | ||
createAppEntrypointWrapper, | ||
|
@@ -67,7 +68,7 @@ const compile = async ({ | |
mode = 'development', | ||
watch = false, | ||
}) => { | ||
const isApp = config.type === 'app' | ||
const isApp = appTypes.includes(config.type) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could export a convenience method .. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated |
||
|
||
verifyEntrypoints({ config, paths }) | ||
if (isApp) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can move LoginAppWrapper into its own directory with supporting hooks, to keep the Login bits contained .. and avoid a lot of branching other than at the top level
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've put it in a separate file. I wasn't sure about putting it in a different directory as it uses a number of the same components (in the same directory) as AppWrapper