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

Issue 1542/refactor to task hooks #1557

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import { CheckBoxOutlined, People } from '@mui/icons-material';

import OverviewListItem from './OverviewListItem';
import { TaskActivity } from 'features/campaigns/models/CampaignActivitiesModel';
import TaskModel from 'features/tasks/models/TaskModel';
import useModel from 'core/useModel';
import useTaskStats from 'features/tasks/hooks/useTaskStats';
import ZUIStackedStatusBar from 'zui/ZUIStackedStatusBar';

interface TasksOverviewListItemProps {
Expand All @@ -17,11 +16,7 @@ const TaskOverviewListItem: FC<TasksOverviewListItemProps> = ({
focusDate,
}) => {
const task = activity.data;
const model = useModel(
(env) => new TaskModel(env, task.organization.id, task.id)
);

const stats = model.getTaskStats().data;
const { data: stats } = useTaskStats(task.organization.id, task.id);

return (
<OverviewListItem
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { CheckBoxOutlined, People } from '@mui/icons-material';

import ActivityListItemWithStats from './ActivityListItemWithStats';
import { STATUS_COLORS } from './ActivityListItem';
import TaskModel from 'features/tasks/models/TaskModel';
import useModel from 'core/useModel';
import useTask from 'features/tasks/hooks/useTask';
import useTaskStats from 'features/tasks/hooks/useTaskStats';
import getTaskStatus, { TASK_STATUS } from 'features/tasks/utils/getTaskStatus';

interface TaskListItemProps {
Expand All @@ -12,9 +12,8 @@ interface TaskListItemProps {
}

const TaskListItem = ({ orgId, taskId }: TaskListItemProps) => {
const model = useModel((env) => new TaskModel(env, orgId, taskId));
const task = model.getTask().data;
const stats = model.getTaskStats().data;
const { data: task } = useTask(orgId, taskId);
const { data: stats, isLoading: statsLoading } = useTaskStats(orgId, taskId);
Comment on lines +15 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

I just wonder why you chose to do this instead of just {data, isLoading} ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

task, stats and statsLoading is used here and there in TaskListItem, and I didn't want to make so many changes to other codes so I unpacked like that but I can change it. No problem!

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, I see now that the renaming of the variables is necessary, well done 💯


if (!task) {
return null;
Expand All @@ -31,8 +30,6 @@ const TaskListItem = ({ orgId, taskId }: TaskListItemProps) => {
color = STATUS_COLORS.BLUE;
}

const statsLoading = model.getTaskStats().isLoading;

return (
<ActivityListItemWithStats
blueChipValue={stats?.assigned}
Expand Down
30 changes: 30 additions & 0 deletions src/features/tasks/hooks/useTask.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { useSelector } from 'react-redux';

import { loadItemIfNecessary } from 'core/caching/cacheUtils';
import { RootState } from 'core/store';
import { ZetkinTask } from '../components/types';
import { taskLoad, taskLoaded } from '../store';
import { useApiClient, useEnv } from 'core/hooks';

interface UseTaskReturn {
data: ZetkinTask | null;
}

export default function useTask(orgId: number, taskId: number): UseTaskReturn {
const apiClient = useApiClient();
const env = useEnv();
const tasks = useSelector((state: RootState) => state.tasks);

const item = tasks.tasksList.items.find((item) => item.id === taskId);

const taskFuture = loadItemIfNecessary(item, env.store, {
actionOnLoad: () => taskLoad(taskId),
actionOnSuccess: (data) => taskLoaded(data),
loader: () =>
apiClient.get<ZetkinTask>(`/api/orgs/${orgId}/tasks/${taskId}`),
});

return {
data: taskFuture.data,
};
}
34 changes: 34 additions & 0 deletions src/features/tasks/hooks/useTaskStats.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { useSelector } from 'react-redux';

import { loadItemIfNecessary } from 'core/caching/cacheUtils';
import { RootState } from 'core/store';
import getStats, { TaskStats } from '../rpc/getTaskStats';
import { statsLoad, statsLoaded } from '../store';
import { useApiClient, useEnv } from 'core/hooks';

interface UseTaskStatsReturn {
data: TaskStats | null;
isLoading: boolean;
}

export default function useTaskStats(
orgId: number,
taskId: number
): UseTaskStatsReturn {
const apiClient = useApiClient();
const env = useEnv();
const tasks = useSelector((state: RootState) => state.tasks);

const item = tasks.statsById[taskId!];

const taskStatsFuture = loadItemIfNecessary(item, env.store, {
actionOnLoad: () => statsLoad(taskId!),
actionOnSuccess: (data) => statsLoaded([taskId!, data]),
loader: () => apiClient.rpc(getStats, { orgId, taskId }),
});

return {
data: taskStatsFuture.data,
isLoading: taskStatsFuture.isLoading,
};
}
28 changes: 28 additions & 0 deletions src/features/tasks/hooks/useTasks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { useSelector } from 'react-redux';

import { IFuture } from 'core/caching/futures';
import { loadListIfNecessary } from 'core/caching/cacheUtils';
import { RootState } from 'core/store';
import { ZetkinTask } from '../components/types';
import { tasksLoad, tasksLoaded } from '../store';
import { useApiClient, useEnv } from 'core/hooks';

interface UseTasksReturn {
tasksFuture: IFuture<ZetkinTask[]>;
}

export default function useTasks(orgId: number): UseTasksReturn {
const taskList = useSelector((state: RootState) => state.tasks.tasksList);
const env = useEnv();
const apiClient = useApiClient();

const tasksFuture = loadListIfNecessary(taskList, env.store, {
actionOnLoad: () => tasksLoad(),
actionOnSuccess: (data) => tasksLoaded(data),
loader: () => apiClient.get<ZetkinTask[]>(`/api/orgs/${orgId}/tasks/`),
});

return {
tasksFuture,
};
}
27 changes: 0 additions & 27 deletions src/features/tasks/models/TaskModel.ts

This file was deleted.

37 changes: 2 additions & 35 deletions src/features/tasks/repos/TasksRepo.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,10 @@
import Environment from 'core/env/Environment';
import IApiClient from 'core/api/client/IApiClient';
import { IFuture } from 'core/caching/futures';
import { loadListIfNecessary } from 'core/caching/cacheUtils';
import { Store } from 'core/store';
import { ZetkinTask } from '../components/types';
import getStats, { TaskStats } from '../rpc/getTaskStats';
import {
loadItemIfNecessary,
loadListIfNecessary,
} from 'core/caching/cacheUtils';
import {
statsLoad,
statsLoaded,
taskLoad,
taskLoaded,
tasksLoad,
tasksLoaded,
} from '../store';
import { tasksLoad, tasksLoaded } from '../store';

export default class TasksRepo {
private _apiClient: IApiClient;
Expand All @@ -26,28 +15,6 @@ export default class TasksRepo {
this._store = env.store;
}

getTask(orgId: number, taskId: number): IFuture<ZetkinTask> {
const state = this._store.getState();
const item = state.tasks.tasksList.items.find((item) => item.id === taskId);

return loadItemIfNecessary(item, this._store, {
actionOnLoad: () => taskLoad(taskId),
actionOnSuccess: (data) => taskLoaded(data),
loader: () =>
this._apiClient.get<ZetkinTask>(`/api/orgs/${orgId}/tasks/${taskId}`),
});
}

getTaskStats(orgId: number, taskId: number): IFuture<TaskStats> {
const state = this._store.getState();
const item = state.tasks.statsById[taskId];
return loadItemIfNecessary(item, this._store, {
actionOnLoad: () => statsLoad(taskId),
actionOnSuccess: (data) => statsLoaded([taskId, data]),
loader: () => this._apiClient.rpc(getStats, { orgId, taskId }),
});
}

getTasks(orgId: number): IFuture<ZetkinTask[]> {
const state = this._store.getState();
const taskList = state.tasks.tasksList;
Expand Down
Loading