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

[TM-1531] add job connection #745

Open
wants to merge 25 commits into
base: staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
aa5b7e6
[TM-1531] add job service url and script
egrojMonroy Dec 11, 2024
153050a
[TM-1531] apiSlice add job resource
egrojMonroy Dec 12, 2024
fa4d2c4
[TM-1531] updates for job service
egrojMonroy Dec 12, 2024
9fbece2
[TM-1531] sort alphabetical
egrojMonroy Dec 12, 2024
8303877
[TM-1531] add connection for list the delayed jobs
egrojMonroy Dec 13, 2024
06783b3
[TM-1531] remove unnecesary code and consistency in selector and API …
egrojMonroy Dec 16, 2024
4972958
[TM-1531] Merge branch 'staging' into feat/TM-1531-add-job-connection
egrojMonroy Dec 16, 2024
e8cef37
[TM-1531] add bulk update call on clear
egrojMonroy Dec 17, 2024
ded0b85
[TM-1531] add entityname and display data in notification
egrojMonroy Dec 17, 2024
c8d7c71
[TM-1531] use store and display only not acknowledged
egrojMonroy Dec 17, 2024
07e2d56
[TM-1531] remove from apifetcher call and add continous call for dela…
egrojMonroy Dec 18, 2024
9d45753
[TM-1531] Merge branch 'staging' into feat/TM-1531-add-job-service
egrojMonroy Dec 18, 2024
05fdae6
[TM-1531] Merge branch 'feat/TM-1531-add-job-service' into feat/TM-15…
egrojMonroy Dec 18, 2024
4637c98
[TM-1531] rollback apifetcher functionality
egrojMonroy Dec 18, 2024
e58c0a1
[TM-1531] adding message to notification
egrojMonroy Dec 19, 2024
996d5a7
[TM-1531] display error messages in notifications
egrojMonroy Dec 19, 2024
b6e0102
[TM-1531] add error messages for failed jobs
egrojMonroy Dec 19, 2024
fc1a52a
[TM-1531] improve connection and remove awaits and asyncs
egrojMonroy Dec 19, 2024
69d08d7
[TM-1531] changes and remove log
egrojMonroy Dec 19, 2024
d5ba353
[TM-1531] Merge branch 'release/utltimate-ulmus' into feat/TM-1531-ad…
egrojMonroy Dec 19, 2024
042ecbc
[TM-1531] yarn generate:services
egrojMonroy Dec 19, 2024
06a7958
[TM-1531] divide names loading for connections
egrojMonroy Dec 20, 2024
6cad9a9
[TM-1531] running with build
egrojMonroy Dec 20, 2024
af9b8da
[TM-1531] only send not pending
egrojMonroy Dec 20, 2024
1b73259
[TM-1531] fix loaded delayedjobs and filter
egrojMonroy Dec 20, 2024
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
8 changes: 8 additions & 0 deletions openapi-codegen.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,33 +27,39 @@ type EnvironmentName = (typeof ENVIRONMENT_NAMES)[number];
type Environment = {
apiBaseUrl: string;
userServiceUrl: string;
jobServiceUrl: string;
entityServiceUrl: string;
};

const ENVIRONMENTS: { [Property in EnvironmentName]: Environment } = {
local: {
apiBaseUrl: "http://localhost:8080",
userServiceUrl: "http://localhost:4010",
jobServiceUrl: "http://localhost:4020",
entityServiceUrl: "http://localhost:4050"
},
dev: {
apiBaseUrl: "https://api-dev.terramatch.org",
userServiceUrl: "https://api-dev.terramatch.org",
jobServiceUrl: "https://api-dev.terramatch.org",
entityServiceUrl: "https://api-dev.terramatch.org"
},
test: {
apiBaseUrl: "https://api-test.terramatch.org",
userServiceUrl: "https://api-test.terramatch.org",
jobServiceUrl: "https://api-test.terramatch.org",
entityServiceUrl: "https://api-test.terramatch.org"
},
staging: {
apiBaseUrl: "https://api-staging.terramatch.org",
userServiceUrl: "https://api-staging.terramatch.org",
jobServiceUrl: "https://api-staging.terramatch.org",
entityServiceUrl: "https://api-staging.terramatch.org"
},
prod: {
apiBaseUrl: "https://api.terramatch.org",
userServiceUrl: "https://api.terramatch.org",
jobServiceUrl: "https://api.terramatch.org",
entityServiceUrl: "https://api.terramatch.org"
}
};
Expand All @@ -66,6 +72,7 @@ if (!ENVIRONMENT_NAMES.includes(declaredEnv as EnvironmentName)) {
const DEFAULTS = ENVIRONMENTS[declaredEnv];
const apiBaseUrl = process.env.NEXT_PUBLIC_API_BASE_URL ?? DEFAULTS.apiBaseUrl;
const userServiceUrl = process.env.NEXT_PUBLIC_USER_SERVICE_URL ?? DEFAULTS.userServiceUrl;
const jobServiceUrl = process.env.NEXT_PUBLIC_JOB_SERVICE_URL ?? DEFAULTS.jobServiceUrl;
const entityServiceUrl = process.env.NEXT_PUBLIC_ENTITY_SERVICE_URL ?? DEFAULTS.entityServiceUrl;

// The services defined in the v3 Node BE codebase. Although the URL path for APIs in the v3 space
Expand All @@ -74,6 +81,7 @@ const entityServiceUrl = process.env.NEXT_PUBLIC_ENTITY_SERVICE_URL ?? DEFAULTS.
// the associated BE code is for a given FE API integration.
const SERVICES = {
"user-service": userServiceUrl,
"job-service": jobServiceUrl,
"entity-service": entityServiceUrl
};

Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
"storybook": "storybook dev -p 6006",
"build-storybook": "storybook build",
"generate:api": "openapi-codegen gen api",
"generate:jobService": "openapi-codegen gen jobService",
"generate:userService": "openapi-codegen gen userService",
"generate:entityService": "openapi-codegen gen entityService",
"generate:services": "yarn generate:userService && yarn generate:entityService",
"generate:services": "yarn generate:userService && yarn generate:entityService && yarn generate:jobService",
"tx:push": "eval $(grep '^TRANSIFEX_TOKEN' .env) && eval $(grep '^TRANSIFEX_SECRET' .env) && txjs-cli push --key-generator=hash src/ --token=$TRANSIFEX_TOKEN --secret=$TRANSIFEX_SECRET",
"tx:pull": "eval $(grep '^TRANSIFEX_TOKEN' .env) && eval $(grep '^TRANSIFEX_SECRET' .env) && txjs-cli pull --token=$TRANSIFEX_TOKEN --secret=$TRANSIFEX_SECRET"
},
Expand Down
19 changes: 2 additions & 17 deletions src/admin/modules/sites/components/SiteShow.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { FC, useState } from "react";
import { FC } from "react";
import { Show, TabbedShowLayout } from "react-admin";

import ShowActions from "@/admin/components/Actions/ShowActions";
import DelayedJobsProgressAlert from "@/admin/components/Alerts/DelayedJobsProgressAlert";
import AuditLogTab from "@/admin/components/ResourceTabs/AuditLogTab/AuditLogTab";
import { AuditLogButtonStates } from "@/admin/components/ResourceTabs/AuditLogTab/constants/enum";
import ChangeRequestsTab from "@/admin/components/ResourceTabs/ChangeRequestsTab/ChangeRequestsTab";
Expand All @@ -16,9 +15,6 @@ import { RecordFrameworkProvider } from "@/context/framework.provider";
import { MapAreaProvider } from "@/context/mapArea.provider";

const SiteShow: FC = () => {
const [isLoadingDelayedJob, setIsLoadingDelayedJob] = useState(false);
const [alertTitle, setAlertTitle] = useState("");

return (
<Show
title={<ShowTitle moduleName="Site" getTitle={record => record?.name} />}
Expand All @@ -30,13 +26,7 @@ const SiteShow: FC = () => {
<InformationTab type="sites" />
<TabbedShowLayout.Tab label="Polygon Review">
<MapAreaProvider>
<PolygonReviewTab
label=""
type={"sites"}
setIsLoadingDelayedJob={setIsLoadingDelayedJob!}
isLoadingDelayedJob={isLoadingDelayedJob!}
setAlertTitle={setAlertTitle!}
/>
<PolygonReviewTab label="" type={"sites"} />
</MapAreaProvider>
</TabbedShowLayout.Tab>
<GalleryTab label="Site Gallery" entity="sites" />
Expand All @@ -46,11 +36,6 @@ const SiteShow: FC = () => {
<AuditLogTab entity={AuditLogButtonStates.SITE} />
</TabbedShowLayout>
</RecordFrameworkProvider>
<DelayedJobsProgressAlert
show={isLoadingDelayedJob}
title={alertTitle}
setIsLoadingDelayedJob={setIsLoadingDelayedJob!}
/>
</Show>
);
};
Expand Down
99 changes: 74 additions & 25 deletions src/components/elements/Notification/FloatNotification.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { LinearProgress } from "@mui/material";
import classNames from "classnames";
import { useState } from "react";
import { useEffect, useState } from "react";
import { When } from "react-if";

import Icon, { IconNames } from "@/components/extensive/Icon/Icon";
import { triggerBulkUpdate, useDelayedJobs } from "@/connections/DelayedJob";
import { DelayedJobData, DelayedJobDto } from "@/generated/v3/jobService/jobServiceSchemas";

import LinearProgressBar from "../ProgressBar/LinearProgressBar/LinearProgressBar";
import Text from "../Text/Text";
Expand All @@ -17,9 +20,33 @@ export interface FloatNotificationProps {
data: FloatNotificationDataProps[];
}

const FloatNotification = ({ data }: FloatNotificationProps) => {
const FloatNotification = () => {
const [openModalNotification, setOpenModalNotification] = useState(false);

const [isLoaded, { delayedJobs }] = useDelayedJobs();
const [notAcknowledgedJobs, setNotAcknowledgedJobs] = useState<DelayedJobDto[]>([]);
const clearJobs = () => {
if (delayedJobs === undefined) return;
const newJobsData: DelayedJobData[] = delayedJobs.map((job: DelayedJobDto) => {
return {
uuid: job.uuid,
type: "delayedJobs",
attributes: {
isAcknowledged: true
}
};
});
triggerBulkUpdate(newJobsData);
};
useEffect(() => {
if (delayedJobs === undefined) return;
const notAcknowledgedJobs = delayedJobs.filter((job: DelayedJobDto) => !job.isAcknowledged);
setNotAcknowledgedJobs(notAcknowledgedJobs);
}, [delayedJobs]);
useEffect(() => {
if (!notAcknowledgedJobs.length) {
setOpenModalNotification(false);
}
}, [notAcknowledgedJobs]);
return (
<div className="fixed bottom-10 right-10 z-50">
<div className="relative">
Expand All @@ -38,36 +65,58 @@ const FloatNotification = ({ data }: FloatNotificationProps) => {
<Text variant="text-14-light" className="text-neutral-400">
Actions Taken
</Text>
<Text variant="text-12-semibold" className="text-primary">
<Text variant="text-12-semibold" className="text-primary" onClick={clearJobs}>
Clear completed
</Text>
</div>
<div className="-mr-2 flex flex-1 flex-col gap-3 overflow-auto pr-2">
{data.map((item, index) => (
<div key={index} className="rounded-lg border-2 border-grey-350 bg-white p-4 hover:border-primary">
<div className="mb-2 flex items-center gap-1">
<div className="h-2 w-2 rounded-full bg-primary" />
<Text variant="text-14-light" className="leading-[normal] text-darkCustom " as={"span"}>
{item.label}
</Text>
</div>
<Text variant="text-14-light" className="text-darkCustom">
Site: <b>{item.site}</b>
</Text>
<div className="mt-2 flex items-center gap-2">
<LinearProgressBar value={parseInt(item.value)} className="h-2 bg-success-40" color="success-600" />
<Text variant="text-12-semibold" className="text-black">
{item.value}
{isLoaded &&
notAcknowledgedJobs &&
notAcknowledgedJobs.map((item, index) => (
<div key={index} className="rounded-lg border-2 border-grey-350 bg-white p-4 hover:border-primary">
<div className="mb-2 flex items-center gap-1">
<div className="h-2 w-2 rounded-full bg-primary" />
<Text variant="text-14-light" className="leading-[normal] text-darkCustom " as={"span"}>
{item.name}
</Text>
</div>
<Text variant="text-14-light" className="text-darkCustom">
Site: <b>{item.entityName}</b>
</Text>
<div className="mt-2 flex items-center gap-2">
{item.name === "Polygon Upload" &&
(item.processedContent === null || item.totalContent === null) &&
item.status === "pending" ? (
<div style={{ width: "100%" }}>
<LinearProgress className="h-2 rounded-full" />
</div>
) : (
<LinearProgressBar
value={
item.status === "succeeded"
? 100
: ((item.processedContent ?? 0) / (item.totalContent ?? 1)) * 100
}
className="h-2 bg-success-40"
color="success-600"
/>
)}
{item.name !== "Polygon Upload" && (
<Text variant="text-12-semibold" className="text-black">
{item.status === "succeeded"
? "100%"
: `${Math.round(((item.processedContent ?? 0) / (item.totalContent ?? 1)) * 100)}%`}
</Text>
)}
</div>
</div>
</div>
))}
))}
</div>
</div>
</div>
<When condition={data.length > 0}>
<When condition={isLoaded && (notAcknowledgedJobs ?? []).length > 0}>
<div className="text-14-bold absolute right-[-5px] top-[-5px] z-20 flex min-h-[24px] min-w-[24px] items-center justify-center rounded-full bg-red-300 leading-[normal] text-white">
{data.length}
{notAcknowledgedJobs?.length}
</div>
</When>
<button
Expand All @@ -77,8 +126,8 @@ const FloatNotification = ({ data }: FloatNotificationProps) => {
className={classNames(
"z-10 flex h-15 w-15 items-center justify-center rounded-full border border-grey-950 bg-primary duration-300 hover:scale-105",
{
hidden: data.length < 1,
visible: data.length > 0
hidden: (notAcknowledgedJobs?.length ?? 0) === 0,
visible: (notAcknowledgedJobs?.length ?? 0) > 0
}
)}
>
Expand Down
103 changes: 103 additions & 0 deletions src/connections/DelayedJob.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { useEffect } from "react";
import { createSelector } from "reselect";

import { bulkUpdateJobs, listDelayedJobs } from "@/generated/v3/jobService/jobServiceComponents";
import {
bulkUpdateJobsFetchFailed,
bulkUpdateJobsIsFetching,
listDelayedJobsFetchFailed
} from "@/generated/v3/jobService/jobServicePredicates";
import { DelayedJobData, DelayedJobDto } from "@/generated/v3/jobService/jobServiceSchemas";
import { ApiDataStore } from "@/store/apiSlice";
import { Connection } from "@/types/connection";
import { connectionHook, connectionLoader } from "@/utils/connectionShortcuts";

// --- Delayed Jobs Connection ---
type DelayedJobsConnection = {
delayedJobs?: DelayedJobDto[];
isLoading: boolean;
hasLoadFailed: boolean;
};

const delayedJobsSelector = (store: ApiDataStore) => {
const delayedJobsMap = store.delayedJobs || {};
roguenet marked this conversation as resolved.
Show resolved Hide resolved
return Object.values(delayedJobsMap).map(resource => resource.attributes);
};

const delayedJobsLoadFailedSelector = (store: ApiDataStore) => {
return listDelayedJobsFetchFailed(store) != null;
};
roguenet marked this conversation as resolved.
Show resolved Hide resolved

const connectionIsLoaded = ({ delayedJobs, hasLoadFailed, isLoading }: DelayedJobsConnection) => {
return (delayedJobs != null && delayedJobs.length > 0) || hasLoadFailed || isLoading;
};
roguenet marked this conversation as resolved.
Show resolved Hide resolved

const delayedJobsConnection: Connection<DelayedJobsConnection> = {
load: connection => {
const isLoaded = connectionIsLoaded(connection);
if (!isLoaded) {
listDelayedJobs();
}
},
isLoaded: connectionIsLoaded,
selector: createSelector(
[delayedJobsSelector, store => delayedJobsLoadFailedSelector(store)],
roguenet marked this conversation as resolved.
Show resolved Hide resolved
(delayedJobs, hasLoadFailed): DelayedJobsConnection => ({
roguenet marked this conversation as resolved.
Show resolved Hide resolved
delayedJobs,
isLoading: delayedJobs == null && !hasLoadFailed,
hasLoadFailed
})
)
};

export const useDelayedJobs = () => {
const connection = connectionHook(delayedJobsConnection)();
roguenet marked this conversation as resolved.
Show resolved Hide resolved

useEffect(() => {
const intervalId = setInterval(() => {
listDelayedJobs();
}, 1500);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will cause this interval to keep refreshing the delayed jobs for as long as the surrounding component is mounted. Is that intentional? It seems like clearing the interval when all the delayed jobs have completed would be good to keep from continuing to hit the jobs endpoint. I'm OK with letting this live to prod if that's complicated to wire up; it's just not ideal and should be cleaned up in a future ticket.

Please either decide to implement an eventual stop like that in this PR, or see that a tech debt ticket is created to clean up in a future release.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes indeed, it is supposed to be attentive for any job created on any session.
Nonetheless I think it might not be good.
So for now the debt ticket will be needed, because we would have to think on the user cases.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the component using this hook expected to be mounted a lot of the time, or does the user have to go to a specific view for it? If this is up all the time, then I think we need to consider a better approach. If it's a view that's only mounted when the user is actively watching for something to finish, then I think this OK for now.


return () => {
clearInterval(intervalId);
};
roguenet marked this conversation as resolved.
Show resolved Hide resolved
}, []);

return connection;
};

export const triggerBulkUpdate = (jobs: DelayedJobData[]) => {
return bulkUpdateJobs({ body: { data: jobs } });
roguenet marked this conversation as resolved.
Show resolved Hide resolved
};

const bulkUpdateJobsSelector = (store: ApiDataStore) => {
const bulkUpdateState = store.delayedJobs || {};
return {
isLoading: bulkUpdateJobsIsFetching(store),
hasLoadFailed: bulkUpdateJobsFetchFailed(store) != null,
response: bulkUpdateState.response
};
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should use ?? for defaults, although I think this is an overall better format for this method:

const bulkUpdateJobsSelector = (store: ApiDataStore) => ({
  isLoading: bulkUpdateJobsIsFetching(store),
  hasLoadFailed: bulkUpdateJobsFetchFailed(store) != null,
  response: store.delayedJobs?.reponse
});

However, I'm also confused as to why you would expect a response object to be on store.delayedJobs. That shouldn't be possible with the current redux setup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the response object wasnt needed, and everything keeps working.


const bulkUpdateJobsConnection: Connection<DelayedJobsConnection, { jobs: DelayedJobData[] }> = {
load: async (connection, { jobs }) => {
const isLoaded = connectionBulkUpdateIsLoaded(connection);
if (!isLoaded) {
await bulkUpdateJobs({ body: { data: jobs } });
roguenet marked this conversation as resolved.
Show resolved Hide resolved
}
},

isLoaded: state => !state.isLoading,

selector: createSelector([store => bulkUpdateJobsSelector(store)], ({ isLoading, hasLoadFailed, response }) => ({
roguenet marked this conversation as resolved.
Show resolved Hide resolved
isLoading,
hasLoadFailed,
response
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like it should be throwing an error in the TS compiler... Maybe I'm missing something, but this shape does not seem to match DelayedJobsConnection.

A couple of thoughts here:

  • Typically, each connection will have its own shape. That's not required, but it's normal
  • I've found in the past that properties like isLoading, hasLoadFailed and response, while seeming convenient when looking at a single connection, are actually a disaster down the road. The reason being that if we follow such generic conventions for individual connections, then when a component ends up using 2 or more connections, it gets really awkward to juggle all the isLoadings that each connection returns. Instead, I think it's better to be super specific with the properties of the connection shape. So something more like bulkUpdateJobsIsLoading and bulkUpdateJobsHasFailed.

As for response - this appears to be just returning all the delayed jobs like the other connection does. Are these used in the same component? Perhaps a better approach would be to combine them into a single connection. I think of connections as being responsible for providing a given set of data from the redux cache, and a set of functions for operating on that data. It would be totally reasonable for this bulkUpdateJobs call to be triggered from a function that's included on the other connection. Or, the bulkUpdate function could be simply exposed by this file, and then we simply rely on the original connection to provide the delayedJobs from the store. One of the primary wins of this system is that a connection returns data that got cached in the store regardless of where it came from. So if one call does the initial fetch, and another call modifies that data, as long as the data ends up back in the store under the same ID, the component that relies on it will rerender when it gets updated.

}))
};

const connectionBulkUpdateIsLoaded = ({ isLoading, hasLoadFailed }: { isLoading: boolean; hasLoadFailed: boolean }) => {
return !isLoading && !hasLoadFailed;
};
roguenet marked this conversation as resolved.
Show resolved Hide resolved

export const useBulkUpdateJobs = connectionLoader(bulkUpdateJobsConnection);
2 changes: 1 addition & 1 deletion src/context/floatNotification.provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const FloatNotificationProvider = ({ children }: FloatNotificationProviderProps)
return (
<FloatNotificationContext.Provider value={value}>
{children}
<FloatNotification data={data} />
<FloatNotification />
</FloatNotificationContext.Provider>
);
};
Expand Down
Loading
Loading