feat(page-services): add jobs #392

RemiBonnet · 2022-11-24T16:57:46Z

What does this PR do?

> Link to the JIRA ticket

Add cron job and lifecycle job on the Services list and Deployment (test here)

PR Checklist

Global

This PR does not introduce any breaking change
This PR introduces breaking change(s) and has been labeled as such
I have found someone to review this PR and pinged him

Store

This PR introduces new store changes

NX

I have run the dep-graph locally and made sure the tree was clean i.e no circular dependencies
I have followed the library pattern i.e feature, ui, data, utils

Clean Code

I made sure the code is type safe (no any)
I have included a feature flag on my feature, if applicable

evoxmusic · 2022-11-24T16:57:49Z

A preview environment was automatically created via Qovery.
Click on the link below to follow its deployment and use it.
👉 [PR] staging - feat(page-services): add jobs - 2022-11-24T16:57:48Z

Another comment will be posted when deployments are terminated

nx-cloud · 2022-11-24T16:57:49Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 74a3f6d. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

evoxmusic · 2022-11-24T17:06:09Z

Your preview environment has been successfully deployed !
Click on the link below to open your service:
👉 console
👉 storybook

codecov-commenter · 2022-11-25T15:49:30Z

Codecov Report

Merging #392 (ba444ab) into staging (94db3e3) will decrease coverage by 4.12%.
The diff coverage is 46.87%.

@@             Coverage Diff             @@
##           staging     #392      +/-   ##
===========================================
- Coverage    57.83%   53.70%   -4.13%     
===========================================
  Files           24      298     +274     
  Lines          268     5554    +5286     
  Branches        92     1179    +1087     
===========================================
+ Hits           155     2983    +2828     
- Misses          79     2189    +2110     
- Partials        34      382     +348

Impacted Files	Coverage Δ
...re/storage-modal-feature/storage-modal-feature.tsx	`61.53% <ø> (ø)`
...plication/src/lib/ui/page-general/page-general.tsx	`50.00% <ø> (ø)`
...c/lib/ui/table-row-services/table-row-services.tsx	`38.46% <29.41%> (ø)`
...e-deployments-feature/page-deployments-feature.tsx	`79.41% <33.33%> (ø)`
...s/application/src/lib/slices/applications.slice.ts	`18.51% <40.00%> (ø)`
...-feature/crud-modal-feature/crud-modal-feature.tsx	`48.57% <100.00%> (ø)`
...ings-ports-feature/page-settings-ports-feature.tsx	`59.09% <100.00%> (ø)`
...ources-feature/page-settings-resources-feature.tsx	`79.41% <100.00%> (ø)`
...-storage-feature/page-settings-storage-feature.tsx	`44.00% <100.00%> (ø)`
...ui/page-deployment-rules/page-deployment-rules.tsx	`20.00% <0.00%> (ø)`
... and 273 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

bdebon

I appreciate the DE improvment with the dataDatabase and others trick.
Now in order to make the whole things easier to read and maintain, I'm proposing an light improvement, tell me what you think about it.

bdebon · 2022-11-28T14:34:54Z

libs/domains/application/src/lib/slices/applications.slice.ts

@@ -183,9 +198,12 @@ export const fetchApplicationDeployments = createAsyncThunk<
  DeploymentHistoryApplication[],
  { applicationId: string; serviceType?: ServiceTypeEnum; silently?: boolean }
 >('application/deployments', async (data) => {
+  console.log(data.serviceType)


bdebon · 2022-11-28T14:37:07Z

libs/domains/application/src/lib/slices/applications.slice.ts

@@ -183,9 +198,12 @@ export const fetchApplicationDeployments = createAsyncThunk<
  DeploymentHistoryApplication[],
  { applicationId: string; serviceType?: ServiceTypeEnum; silently?: boolean }
 >('application/deployments', async (data) => {
+  console.log(data.serviceType)
  let response
  if (data.serviceType === ServiceTypeEnum.CONTAINER) {


For these 3 conditions can we have an util somewhere that contains these 3 methods:
isContainer()
isJob()
isApplication() ?

Like this it would be factorised somewhere and if one day the condition to detect the type changes, we just have to change it in one place. What do you think?

bdebon · 2022-11-28T14:44:00Z

libs/pages/services/src/lib/ui/table-row-services/table-row-services.tsx

-              <Icon name={type === ServiceTypeEnum.DATABASE ? IconEnum.DATABASE : IconEnum.APPLICATION} width="20" />
+              <Icon
+                name={
+                  type === ServiceTypeEnum.APPLICATION || type === ServiceTypeEnum.CONTAINER


With this multiplication of rather complex tests here and there, I think even more about the util that contains isContainer() isGitApplication() isDatabase() isJob()
It would be much comfortable to read and to maintain no?

bdebon · 2022-11-28T14:45:24Z

libs/pages/services/src/lib/ui/table-row-services/table-row-services.tsx

@@ -108,25 +116,34 @@ export function TableRowServices(props: TableRowServicesProps) {
          <Skeleton className="w-full" show={isLoading} width={160} height={16}>
            <div className="w-full flex gap-2 items-center -mt-[1px]">
              {type === ServiceTypeEnum.APPLICATION && (


Because with just these conditions we have to make the effort everytime to see if you test if it's an gitapp, a container app, etc... Frontend joining us could be lost quickly and so could we after many months

bdebon · 2022-11-28T14:46:26Z

libs/pages/services/src/lib/ui/table-row-services/table-row-services.tsx

@@ -45,33 +46,39 @@ export function TableRowServices(props: TableRowServicesProps) {
    isLoading,
  } = props

+  const dataDatabase = data as DatabaseEntity


I like this

bdebon · 2022-11-28T14:48:55Z

libs/shared/enums/src/lib/utils/get-service-type.ts

 } from '@qovery/shared/interfaces'
 import { ServiceTypeEnum } from '../service-type.enum'

 export const getServiceType = (data: ApplicationEntity | DatabaseEntity) => {
  let currentType: ServiceTypeEnum

+  const isJob = (data as JobApplicationEntity).max_nb_restart
+


Ok so basically this file already exists, I think you could add some utils in this one that are: isDatabase(data) and that basically would check that getServiceType(data) === ServiceType.DATABASE you see?

bdebon · 2022-11-28T14:49:31Z

libs/shared/interfaces/src/lib/domain/job-application.entity.ts

+import { LoadingStatus } from '../types/loading-status.type'
+import { ServiceRunningStatus } from './service-running-status.interface'
+
+export interface JobApplicationEntity extends JobResponse {


Why not JobEntity ?

Because it's the same pattern with other application

job-application.entity.ts
git-application.entity.ts
container-application.entity.ts

Well, others are really applications, a git and a container give an application. For me, a Job is a Job, but anyway, it's not a big problem, it's totally fine like this too.

bdebon

This refactoring is a bless! Thanks Remi, I'm very happy with this PR!

bdebon · 2022-11-28T16:38:09Z

libs/shared/enums/src/lib/utils/get-service-type.ts

  } else {
    currentType = ServiceTypeEnum.APPLICATION
  }

  return currentType
 }
+
+// Job
+export const isJob = (data?: ApplicationEntity | ServiceTypeEnum) => {


Perfecto me gusta mucho!!

bdebon · 2022-11-28T16:40:01Z

libs/shared/interfaces/src/lib/domain/job-application.entity.ts

+import { LoadingStatus } from '../types/loading-status.type'
+import { ServiceRunningStatus } from './service-running-status.interface'
+
+export interface JobApplicationEntity extends JobResponse {


Well, others are really applications, a git and a container give an application. For me, a Job is a Job, but anyway, it's not a big problem, it's totally fine like this too.

bdebon · 2022-11-30T12:56:45Z

🎉 This PR is included in version 1.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

wip

8d225c4

wip

1be03ab

RemiBonnet added 2 commits November 25, 2022 14:18

wip

e4cb967

wip

91e234f

RemiBonnet added 2 commits November 25, 2022 17:12

fix

303511c

update

8ece8b2

RemiBonnet requested a review from bdebon November 28, 2022 14:33

RemiBonnet marked this pull request as ready for review November 28, 2022 14:36

fix

ba444ab

bdebon suggested changes Nov 28, 2022

View reviewed changes

wip

7de803d

bdebon approved these changes Nov 28, 2022

View reviewed changes

RemiBonnet added 2 commits November 28, 2022 18:19

wip

5b3edae

wip

74a3f6d

RemiBonnet merged commit eee4336 into staging Nov 28, 2022

RemiBonnet deleted the feat/jobs-services-page-FRT-588 branch November 28, 2022 17:38

bdebon added the released label Nov 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(page-services): add jobs #392

feat(page-services): add jobs #392

RemiBonnet commented Nov 24, 2022 •

edited

evoxmusic commented Nov 24, 2022

nx-cloud bot commented Nov 24, 2022 •

edited

evoxmusic commented Nov 24, 2022

codecov-commenter commented Nov 25, 2022 •

edited

bdebon left a comment

bdebon Nov 28, 2022

bdebon Nov 28, 2022

bdebon Nov 28, 2022

bdebon Nov 28, 2022

bdebon Nov 28, 2022

bdebon Nov 28, 2022

bdebon Nov 28, 2022

RemiBonnet Nov 28, 2022

bdebon Nov 28, 2022

bdebon left a comment

bdebon Nov 28, 2022

bdebon Nov 28, 2022

bdebon commented Nov 30, 2022

feat(page-services): add jobs #392

feat(page-services): add jobs #392

Conversation

RemiBonnet commented Nov 24, 2022 • edited

What does this PR do?

PR Checklist

Global

Store

NX

Clean Code

evoxmusic commented Nov 24, 2022

nx-cloud bot commented Nov 24, 2022 • edited

☁️ Nx Cloud Report

evoxmusic commented Nov 24, 2022

codecov-commenter commented Nov 25, 2022 • edited

Codecov Report

bdebon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdebon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdebon commented Nov 30, 2022

RemiBonnet commented Nov 24, 2022 •

edited

nx-cloud bot commented Nov 24, 2022 •

edited

codecov-commenter commented Nov 25, 2022 •

edited