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

Better import in progress #2027

Merged
merged 4 commits into from
Mar 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions src/AppContainer.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* global __TARGET__ */

import React from 'react'
import React, { useMemo } from 'react'
import { Provider } from 'react-redux'

import I18n from 'cozy-ui/transpiled/react/I18n'
Expand All @@ -12,23 +12,45 @@ import { BreakpointsProvider } from 'cozy-ui/transpiled/react/hooks/useBreakpoin
import flag from 'cozy-flags'

import { TrackerProvider } from 'ducks/tracking/browser'
import JobsProvider from 'components/JobsContext'
import Alerter from 'cozy-ui/transpiled/react/Alerter'
import { initTranslation } from 'cozy-ui/react/I18n'

const jobsProviderOptions = t => ({
onSuccess: () => Alerter.success(t('JobsContext.alerter-success')),
onError: () => Alerter.error(t('JobsContext.alerter-errored'))
})

const initT = (lang, dictRequire) => {
const polyglot = initTranslation(lang, dictRequire)
const t = polyglot.t.bind(polyglot)
return { t }
}

const AppContainer = ({ store, lang, history, client }) => {
const AppRoute = require('components/AppRoute').default
const Router =
__TARGET__ === 'mobile' || flag('authentication')
? require('ducks/mobile/MobileRouter').default
: require('react-router').Router

const dictRequire = lang => require(`locales/${lang}`)
const { t } = useMemo(() => {
return initT(lang, dictRequire)
}, [lang])

return (
<BreakpointsProvider>
<IconSprite />
<TrackerProvider>
<Provider store={store}>
<CozyProvider client={client}>
<I18n lang={lang} dictRequire={lang => require(`locales/${lang}`)}>
<MuiCozyTheme>
<Router history={history} routes={AppRoute()} />
</MuiCozyTheme>
<I18n lang={lang} dictRequire={dictRequire}>
<JobsProvider client={client} options={jobsProviderOptions(t)}>
<MuiCozyTheme>
<Router history={history} routes={AppRoute()} />
</MuiCozyTheme>
</JobsProvider>
</I18n>
</CozyProvider>
</Provider>
Expand Down
2 changes: 1 addition & 1 deletion src/components/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { withRouter } from 'react-router'
import compose from 'lodash/flowRight'

import Alerter from 'cozy-ui/transpiled/react/Alerter'
import { Layout, Main, Content } from 'cozy-ui/transpiled/react/Layout'
import { Content, Layout, Main } from 'cozy-ui/transpiled/react/Layout'
import UISidebar from 'cozy-ui/transpiled/react/Sidebar'

import { settingsConn } from 'doctypes'
Expand Down
127 changes: 127 additions & 0 deletions src/components/JobsContext.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/* eslint-disable react-hooks/exhaustive-deps */
import React, {
createContext,
useContext,
useEffect,
useMemo,
useRef,
useState
} from 'react'
import { JOBS_DOCTYPE } from '../doctypes'
import CozyRealtime from 'cozy-realtime'
import logger from 'cozy-logger'
import isFunction from 'lodash/isFunction'

const log = logger.namespace('import.context')

export const JobsContext = createContext({})

export const useJobsContext = () => {
return useContext(JobsContext)
}

// @TODO add tests for JobsContext
/** Allows to subscribe to jobs and thus to know jobs in progress
*
* @param client
* @param options
* @returns jobsInProgress
*/
const JobsProvider = ({ children, client, options }) => {
const [jobsInProgress, setJobsInProgress] = useState([])
const realtimeStartedRef = useRef(false)
const jobsInProgressRef = useRef([])

const realtime = useMemo(() => {
return new CozyRealtime({ client })
}, [client])

const handleRealtime = data => {
const { worker, state, message: msg } = data

// hack: Using jobsInProgressRef because jobsInProgress (from state)
// keep its default value
// The ideal would be to use only jobsInProgress and do not use the ref
const { current: currJobsInProgress } = jobsInProgressRef
ptbrowne marked this conversation as resolved.
Show resolved Hide resolved
const index = currJobsInProgress.findIndex(a => a.account === msg.account)
const exist = index !== -1

const { onSuccess, onError } = options
let arr = [...currJobsInProgress]
if (worker === 'konnector') {
if (state === 'running' && !exist) {
arr.push(msg)
} else if (state === 'done' && exist) {
arr.splice(index, 1)

if (isFunction(onSuccess)) {
onSuccess()
}
} else if (state === 'errored' && exist) {
arr.splice(index, 1)
if (isFunction(onError)) {
onError()
}
}
ptbrowne marked this conversation as resolved.
Show resolved Hide resolved
jobsInProgressRef.current = arr
setJobsInProgress(jobsInProgressRef.current)
}
}

const startJobsRealtime = () => {
if (!realtimeStartedRef.current) {
log('info', 'Start Jobs Realtime')
realtimeStartedRef.current = true

realtime.subscribe('created', JOBS_DOCTYPE, handleRealtime)
realtime.subscribe('updated', JOBS_DOCTYPE, handleRealtime)
realtime.subscribe('deleted', JOBS_DOCTYPE, handleRealtime)
}
}

const stopJobsRealtime = () => {
if (realtimeStartedRef.current) {
log('info', 'Stop Jobs Realtime')
realtimeStartedRef.current = false

realtime.unsubscribe('created', JOBS_DOCTYPE, handleRealtime)
realtime.unsubscribe('updated', JOBS_DOCTYPE, handleRealtime)
realtime.unsubscribe('deleted', JOBS_DOCTYPE, handleRealtime)
}
}

// @TODO useRealtime hook
useEffect(() => {
startJobsRealtime()
return () => stopJobsRealtime()
}, [])
Copy link
Contributor

Choose a reason for hiding this comment

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

There is useRealtime hook that could simplify the logic, it does the start/stop/subscribe/unsubscribe for you:

https://github.com/cozy/cozy-ui/blob/db54f27493b6ba61cf41a1b48cf363b9957dd049/react/hooks/useRealtime.js

cozy-ui might not be the good place for it though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I think so too, it would be better in realtime lib


return (
<JobsContext.Provider
value={{
jobsInProgress
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: It could be nice to prepare a bit the context value to make its consumption easier for the components below. For example, indexing the accounts or the triggers, or the connectors.

  const value = useMemo(() => ({
    inProgress: jobsInProgress,
    byAccountId: keyBy(jobsInProgress, x => x.account),
    byKonnectorSlug: keyBy(jobsInProgress, x => x.konnector),,
    byTriggerId: keyBy(jobsInProgress, x => x.trigger), // useMemo could be used 
  }, [jobsInProgress])
  return <JobsContext.Provider
    value={value}
  ...
  />

The value passed below is here memoized not to incure unnecessary renders (not sure if this is really necessary but it cannot hurt I guess) and indexed so that a component can access info easily.

}}
>
{children}
</JobsContext.Provider>
)
}

export default JobsProvider

/**
* @function
* @description HOC to provide import context as prop
*
* @param {Component} Component - wrapped component
* @returns {Function} - Component that will receive import context as prop
*/
export const withJobsInProgress = Component => {
const Wrapped = props => {
const { jobsInProgress = [] } = useJobsContext()
return <Component {...props} jobsInProgress={jobsInProgress} />
}
Wrapped.displayName = `withJobsInProgress(${Component.displayName ||
Component.name})`
return Wrapped
}
6 changes: 6 additions & 0 deletions src/doctypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const TRANSACTION_DOCTYPE = 'io.cozy.bank.operations'
export const SETTINGS_DOCTYPE = 'io.cozy.bank.settings'
export const BILLS_DOCTYPE = 'io.cozy.bills'
export const TRIGGER_DOCTYPE = 'io.cozy.triggers'
export const JOBS_DOCTYPE = 'io.cozy.jobs'
export const APP_DOCTYPE = 'io.cozy.apps'
export const KONNECTOR_DOCTYPE = 'io.cozy.konnectors'
export const COZY_ACCOUNT_DOCTYPE = 'io.cozy.accounts'
Expand Down Expand Up @@ -249,3 +250,8 @@ export const myselfConn = {
as: 'myself',
fetchPolicy: older30s
}

export const konnectorConn = {
query: slug => Q(KONNECTOR_DOCTYPE).getById(`${KONNECTOR_DOCTYPE}/${slug}`),
as: 'konnector'
}
2 changes: 1 addition & 1 deletion src/ducks/balance/AccountRowLoading.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class AccountRowLoading extends React.PureComponent {
<Typography variant="body1">
{isErrored
? t('Balance.importing-accounts-error')
: t('Balance.importing-accounts')}
: t('Balance.import-accounts')}
</Typography>
<Typography variant="caption">
{isErrored ? (
Expand Down
2 changes: 1 addition & 1 deletion src/ducks/balance/AccountRowLoading.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('Account Row Loading', () => {

it('should progress with any status', () => {
const { root } = setup(konnectorInfos[0])
expect(root.getByText('Importing accounts')).toBeTruthy()
expect(root.getByText('Import accounts')).toBeTruthy()
expect(root.getByText('In progress')).toBeTruthy()
expect(root.getByRole('progressbar')).toBeTruthy()
})
Expand Down
10 changes: 7 additions & 3 deletions src/ducks/balance/AccountsImporting.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import BarTheme from 'ducks/bar/BarTheme'

import headerTitleStyles from 'ducks/balance/HeaderTitle.styl'
import styles from 'ducks/balance/AccountsImporting.styl'
import flag from 'cozy-flags'
import Delayed from 'components/Delayed'

const muiStyles = () => ({
linearColorPrimary: {
Expand Down Expand Up @@ -95,20 +97,22 @@ const AccountsImporting = ({ classes, konnectorInfos }) => {
)
}

const groupPanelDelay = flag('balance.no-delay-groups') ? 0 : 150

const ImportInProgress = ({ classes }) => {
const { t } = useI18n()
return (
<>
<Delayed delay={groupPanelDelay}>
<LinearProgress
className={styles.progress}
classes={{
colorPrimary: classes.linearColorPrimary,
barColorPrimary: classes.linearBarColorPrimary
}}
/>
<div className={styles.text}>{t('Balance.importing-accounts')}</div>
<div className={styles.text}>{t('Balance.import-accounts')}</div>
<div className={styles.text}>{t('Balance.delay')}</div>
</>
</Delayed>
)
}

Expand Down
4 changes: 2 additions & 2 deletions src/ducks/balance/AccountsImporting.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('Importing Accounts', () => {
konnectorInfos
})
expect(root.getAllByRole('progressbar').length).toEqual(3)
expect(root.getAllByText('Importing accounts').length).toEqual(3)
expect(root.getAllByText('Import accounts').length).toEqual(3)
expect(root.getByText('This may take a few minutes…')).toBeTruthy()
})

Expand All @@ -60,7 +60,7 @@ describe('Importing Accounts', () => {
})

expect(root.queryAllByRole('progressbar').length).toEqual(0)
expect(root.queryByText('Importing accounts')).toBeFalsy()
expect(root.queryByText('Import accounts')).toBeFalsy()
expect(root.queryByText('This may take a few minutes…')).toBeFalsy()
})
})
6 changes: 5 additions & 1 deletion src/ducks/balance/Balance.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ import BarTheme from 'ducks/bar/BarTheme'
import { filterByAccounts } from 'ducks/filters'
import { trackPage } from 'ducks/tracking/browser'
import { isVirtualAccount } from 'ducks/balance/helpers'
import ImportGroupPanel from 'ducks/balance/ImportGroupPanel'
import { withJobsInProgress } from 'components/JobsContext'

const syncPouchImmediately = async client => {
const pouchLink = client.links.find(link => link.pouches)
Expand Down Expand Up @@ -415,6 +417,7 @@ class Balance extends PureComponent {
[styles.Balance__panelsContainer]: true
})}
>
<ImportGroupPanel />
<BalancePanels
groups={groups}
panelsState={this.state.panels}
Expand Down Expand Up @@ -451,5 +454,6 @@ export default compose(
virtualGroups: getVirtualGroups
})
),
withClient
withClient,
withJobsInProgress
)(Balance)
2 changes: 1 addition & 1 deletion src/ducks/balance/Balance.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ describe('Balance page', () => {
expect(wrapper.find(NoAccount)).toHaveLength(1)
})

it('should show importing accounts', () => {
it('should show import accounts', () => {
const triggers = [
{
attributes: {
Expand Down
4 changes: 2 additions & 2 deletions src/ducks/balance/GroupPanel.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import BottomIcon from 'cozy-ui/transpiled/react/Icons/Bottom'

import Typography from 'cozy-ui/transpiled/react/Typography'

const GroupPanelSummary = withStyles({
export const GroupPanelSummary = withStyles({
root: {
maxHeight: '3.5rem',
height: '3.5rem'
Expand All @@ -53,7 +53,7 @@ const GroupPanelSummary = withStyles({
}
})(ExpansionPanelSummary)

const GroupPanelExpandIcon = React.memo(function GroupPanelExpandIcon() {
export const GroupPanelExpandIcon = React.memo(function GroupPanelExpandIcon() {
return (
<Icon
icon={BottomIcon}
Expand Down
Loading