feat(profile): implement employment history #2259

ThorsAngerVaNeT · 2023-08-13T18:14:26Z

🟢 Add deploy label if you want to deploy this Pull Request to staging environment

🧑‍⚖️ Pull Request Naming Convention

Title should follow Conventional Commits
Do not put issue id in title
Do not put WIP in title. Use Draft PR functionality
Consider to add area:* label(s)

I followed naming convention rules

🤔 This is a ...

🔗 Related issue link

#2202

💡 Background and solution

Add employment history block to profile so students could submit in the app their working experience.

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

Database migration is added or not needed
Documentation is updated/provided or not needed
Changes are tested locally

… openapi

…mitted false

server/src/models/user.ts

client/src/components/Profile/JobFoundButtonWithModal.tsx

…e with updating previous

…terface

…t record and back

…for employment records

This reverts commit fcdd600.

This reverts commit 81a7969.

…methods for employment records" This reverts commit 13c2ddf.

client/src/components/Profile/MainCard.tsx

I-vasilich-I · 2023-10-14T06:30:56Z

client/src/services/user.ts

 export type ProfileMainCardData = {
  location: Location | null;
  name: string;
  githubId: string | null;
  publicCvUrl: string | null;
+  employmentHistory: EmploymentRecordDto[] | undefined;


Looks like you don't need employmentHistory on the MainCardData. AFAIK MainCard do not use it.

I-vasilich-I · 2023-10-14T06:32:48Z

client/src/services/user.ts

@@ -230,13 +237,15 @@ export type ProfileInfo = {
  publicFeedback?: PublicFeedback[];
  stageInterviewFeedback?: StageInterviewDetailedFeedback[];
  discord: Discord | null;
+  employmentHistory: EmploymentRecordDto[] | undefined;


Do you need explicit undefind or optional would suffice?

I-vasilich-I · 2023-10-14T06:35:58Z

client/src/pages/profile/index.tsx

@@ -193,6 +200,7 @@ export class ProfilePage extends React.Component<Props, State> {
      name: profile?.generalInfo?.name ?? '',
      githubId: profile?.generalInfo?.githubId ?? null,
      publicCvUrl: profile?.publicCvUrl ?? null,
+      employmentHistory: profile?.employmentHistory,


Looks like you don't need employmentHistory on the mainInfo. AFAIK MainCard do not use it.

I-vasilich-I · 2023-10-14T06:39:40Z

client/src/pages/profile/index.tsx

@@ -162,6 +168,7 @@ export class ProfilePage extends React.Component<Props, State> {
        profile: {
          ...profile,
          publicCvUrl: profile?.publicCvUrl ?? null,
+          employmentHistory: profile?.employmentHistory,


if employmentHistory is optional you don't need to set it here too.

client/src/components/Profile/EmploymentCard.tsx

I-vasilich-I · 2023-10-14T06:57:35Z

client/src/components/Profile/EmploymentHistory/EmploymentHistoryDisplayItem.tsx

+      <Title level={5}>
+        {title} at {companyName}
+      </Title>
+      {dateFrom && dateTo && <Text type="secondary">{getWorkingPeriod(dateFrom, dateTo)}</Text>}


If you set currently working at some company, dateTo is not set yet(need to reload page, see comment on EmploymentCard), therefore this line won't show expected dateFrom - Present - duration.

I'd handle empty dateTo in getWorkingPeriod

I-vasilich-I · 2023-10-14T07:04:58Z

client/src/components/Profile/EmploymentHistory/EmploymentHistoryFormItem.tsx

+          label="Position title"
+          rules={[{ required: true, message: '${label} is required' }]}
+        >
+          <Input />


Could you add placeholder on inputs?

client/src/components/Profile/EmploymentHistory/EmploymentHistoryFormItem.tsx

I-vasilich-I · 2023-10-14T08:13:19Z

client/src/components/Profile/EmploymentCard.tsx

+      return;
+    }
+
+    setDisplayEmployments(employments);


related to EmploymentHistoryDisplayItem comment
So after save, if I don't reload the page I won't see employment from-present-duration, because it relies on dateTo to be truthy,

this could solve it:

setDisplayEmployments(employments.map(( item ) => ({...item, dateTo: item.dateTo ? dayjs(item.dateTo) : dayjs() })));

but IMHO the way you set dateTo on employmentRecordDtoToFormItem seams not right, if it's not set, it should stay not set

better to handle it on EmploymentHistoryDisplayItem

this could solve it:

setDisplayEmployments(employments.map(( item ) => ({...item, dateTo: item.dateTo ? dayjs(item.dateTo) : dayjs() })));

Yeah, I had such solution before somewhere, but for some reason I get rid of it.
Thanks I will double check this issue

client/src/components/Profile/EmploymentCard.tsx

aaliakseyenka · 2023-10-16T16:22:50Z

server/src/models/user.ts

@@ -33,6 +33,7 @@ export interface EmploymentRecord {
  dateFrom: string;
  companyName: string;
  toPresent: boolean;
+  officeLocation?: Location;


maybe just location?

nestjs/src/profile/profile.service.ts

aaliakseyenka · 2023-10-17T07:34:01Z

client/src/api/api.ts

+     * @type {object}
+     * @memberof EmploymentRecordDto
+     */
+    'officeLocation'?: object;


should it be object here?

client/src/components/Profile/EmploymentCard.tsx

aaliakseyenka · 2023-10-17T07:37:44Z

client/src/components/Profile/EmploymentCard.tsx

+  const [displayEmployments, setDisplayEmployments] = useState<EmploymentRecordFormItem[]>(
+    data.map(employmentRecordDtoToFormItem),
+  );
+  const [employments, setEmployments] = useState<EmploymentRecordFormItem[]>(displayEmployments);


IMO looks weird to have 2 states for the same data

aaliakseyenka · 2023-10-17T07:39:00Z

common/models/profile.ts

+export interface JobFoundInfo {
+  jobFound: boolean;
+  jobFoundCompanyName?: string | null;
+  jobFoundOfficeLocation?: string | null;


do you really need to repeat this jobFound in each property?

aaliakseyenka · 2023-10-17T07:42:03Z

client/src/components/Profile/EmploymentHistory/EmploymentHistoryFormItem.tsx

+
+const EmploymentHistoryFormItem = ({ name, restField, remove, form }: EmploymentHistoryFormItemPros) => {
+  const { title, dateFrom, dateTo, toPresent, companyName, officeLocation } =
+    form.getFieldValue(['employmentHistory', name]) ?? {};


why do you need explicitly get this data rather than us form items to consume it automatically?

aaliakseyenka · 2023-10-17T07:44:10Z

client/src/components/Profile/EmploymentHistory/EmploymentHistoryFormItem.tsx

+    form.getFieldValue(['employmentHistory', name]) ?? {};
+  const [isToPresent, setToPresent] = useState<boolean>(toPresent);
+  const [isDateToRequired, setIsDateToRequired] = useState<boolean>(!toPresent);
+  const [locationSelectValue, setLocationSelectValue] = useState(officeLocation);


could u. please clarify why do you need state for them. why antd form state is not sufficient?

client/src/components/Profile/EmploymentHistory/EmploymentHistoryDisplayItem.tsx

…ange

ThorsAngerVaNeT added 15 commits August 9, 2023 21:05

feat(models): add jobFound related fields to User model

81a7969

feat(models): add migration for User model

fcdd600

feat(profile): add JobFoundDto

438eb45

fix(models): formatting

ee9cb74

feat(profile): add updateJobFound endpoint

7c2a02e

feat(profile): add get job-found endpoint

2b004bf

fix(profile): decorator in controller and type in JobFoundDto, update…

2701bd3

… openapi

feat(profile): add JobFoundButtonWithModal component

0c087bb

fix(profile): JobFoundDto and returning value for GET job-found

f3f0a91

fix(profile): inputs type possibility

b3155fb

feat(profile): add ModalSubmitForm and submit request

d8ed3a9

feat(profile): send GET job-found request only when isProfileOwner

785597f

refactor: delete multiple empty lines

0dfcc1c

feat(profile): clear companyName and officeLocation when checkbox sub…

b340a59

…mitted false

refactor(profile): make JobFoundButtonAndModal self-closed

67d3899

ThorsAngerVaNeT changed the title ~~Feature: add job found button to profile so students could submit~~ Feature: add job found button to profile Aug 13, 2023

Merge remote-tracking branch 'upstream/master' into feat/job-found

3b80353

apalchys changed the title ~~Feature: add job found button to profile~~ feat: add job found button to profile Aug 16, 2023

Merge branch 'master' into feat/job-found

c06adfd

aaliakseyenka reviewed Aug 26, 2023

View reviewed changes

server/src/models/user.ts Outdated Show resolved Hide resolved

apalchys reviewed Sep 6, 2023

View reviewed changes

client/src/components/Profile/JobFoundButtonWithModal.tsx Outdated Show resolved Hide resolved

ThorsAngerVaNeT added 4 commits September 9, 2023 13:08

feat(users): add methods to get last employment record and add new on…

9e58a0b

…e with updating previous

feat(users): add optional officeLocation field to EmploymentRecord in…

227bf4d

…terface

feat(job-found): add methods to transform job found info to employmen…

facd174

…t record and back

feat(job-found): adapt job found methods to use UsersService methods …

13c2ddf

…for employment records

ThorsAngerVaNeT marked this pull request as draft September 9, 2023 11:50

ThorsAngerVaNeT added 4 commits September 10, 2023 20:04

Revert "feat(models): add migration for User model"

17803f0

This reverts commit fcdd600.

Revert "feat(models): add jobFound related fields to User model"

2364eac

This reverts commit 81a7969.

Merge remote-tracking branch 'upstream/master' into feat/job-found

e399c3d

Revert "feat(job-found): adapt job found methods to use UsersService …

00ae86d

…methods for employment records" This reverts commit 13c2ddf.

ThorsAngerVaNeT added 8 commits September 25, 2023 09:21

Merge branch 'master' into feat/job-found

3a4338f

feat(api): update openapi

8264310

feat(profile): add nested validation for dto

927a5e7

feat(profile): add ceil round for working duration like in LinkedIn

69f60a8

feat(profile): add validation for dateTo

fa05e4f

fix(profile): formatting

299456c

fix(profile): dateTo validator get wrong dateFrom value

b14ebca

fix(profile): validation run earlier than form create

6b2cc9c

ThorsAngerVaNeT changed the title ~~feat: add job found button to profile~~ feat(profile): implement employment history Sep 26, 2023

ThorsAngerVaNeT marked this pull request as ready for review September 26, 2023 05:41

ThorsAngerVaNeT requested review from valerydluski, apalchys and aaliakseyenka September 26, 2023 15:53

I-vasilich-I reviewed Oct 14, 2023

View reviewed changes

ThorsAngerVaNeT added 2 commits October 16, 2023 08:36

refactor: delete redundant empty parent elements

fa76eae

Merge branch 'master' into feat/job-found

c9f9d50

valerydluski reviewed Oct 16, 2023

View reviewed changes

client/src/components/Profile/EmploymentCard.tsx Outdated Show resolved Hide resolved

refactor: remove redundant React import

c32cced

aaliakseyenka reviewed Oct 17, 2023

View reviewed changes

ThorsAngerVaNeT added 2 commits October 29, 2023 22:02

refactor: delete redundant interface

8f38edd

refactor(employment-history): move function outside component

acdc5fe

ThorsAngerVaNeT marked this pull request as draft October 29, 2023 19:11

ThorsAngerVaNeT added 7 commits October 29, 2023 22:18

refactor(employment-history): select only needed field from DB

a5edb54

refactor(employment-history): delete unnecessary field definition

f920753

feat(employment-history): add requiredMark optional to Form

cc85376

feat(employment-history): add dateFrom value validation

2a7eee4

feat(employment-history): add date fields revalidation on checkbox ch…

92c3cde

…ange

Merge branch 'master' into feat/job-found

12ecd78

refactor: rename officeLocation field

3e49315

ThorsAngerVaNeT closed this Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(profile): implement employment history #2259

feat(profile): implement employment history #2259

ThorsAngerVaNeT commented Aug 13, 2023 •

edited

Loading

I-vasilich-I Oct 14, 2023

I-vasilich-I Oct 14, 2023

I-vasilich-I Oct 14, 2023

I-vasilich-I Oct 14, 2023

I-vasilich-I Oct 14, 2023

I-vasilich-I Oct 14, 2023

I-vasilich-I Oct 14, 2023

ThorsAngerVaNeT Oct 16, 2023

aaliakseyenka Oct 16, 2023

aaliakseyenka Oct 17, 2023

aaliakseyenka Oct 17, 2023

aaliakseyenka Oct 17, 2023

aaliakseyenka Oct 17, 2023

aaliakseyenka Oct 17, 2023

feat(profile): implement employment history #2259

feat(profile): implement employment history #2259

Conversation

ThorsAngerVaNeT commented Aug 13, 2023 • edited Loading

🧑‍⚖️ Pull Request Naming Convention

🤔 This is a ...

🔗 Related issue link

💡 Background and solution

☑️ Self Check before Merge

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

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

ThorsAngerVaNeT commented Aug 13, 2023 •

edited

Loading