TASK-304 Another iteration over drawer for managing workers #279

Qwebeck · 2021-03-13T23:58:53Z

Poprawiłem drawer do edycji/dodawania pracownika w taki sposób, żeby działała konwersja godzin oraz dodałem komunikaty o błędach.

@Ehevi zerknij proszę na komunikaty które pojawiają się przy błędach i powiedz czy masz jakiś uwagi
@VVlovsky , @maciekb05 zerknijcie proszę od strony kodu

…nput

cypress · 2021-03-14T00:02:49Z

Test summary

232 • 0 • 0 • 0 • 0

Run details


Project	NurseScheduling
Status	Passed
Commit	`7f56a2d`
Started	Mar 19, 2021 3:02 PM
Ended	Mar 19, 2021 3:06 PM
Duration	03:47 💡
OS	Linux Ubuntu - 20.04
Browser	Custom chrome 88

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

src/components/namestable/use-worker-info.ts

src/state/reducers/month-state/employee-info.reducer.ts

src/components/namestable/worker-edit/worker-edit.component.tsx

Qwebeck · 2021-03-14T00:05:28Z

src/components/namestable/worker-edit/worker-civil-contract-work-norm-selector.component.tsx

+        />
+        <FormFieldErrorLabel
+          condition={!isTimeValid()}
+          message={`Ilość godzin powinna być mniejsza od ${maximumWorkHoursForMonth}`}


Liczba godzin. Godziny są policzalne 😝

src/components/namestable/worker-edit/worker-contract-type-selector.component.tsx

Qwebeck · 2021-03-14T00:06:13Z

...mponents/namestable/worker-edit/worker-employement-contract-work-norm-selector.component.tsx

+          />
+          <FormFieldErrorLabel
+            condition={!isEmployementTimeValid(employementTime)}
+            message="Etat powinien być mniejszy lub równy jeden"


src/components/namestable/worker-edit/worker-name-edit-field.components.tsx

src/components/namestable/worker-edit/worker-position-selector.component.tsx

Ehevi

liczba godzin (skoro godziny liczymy, to są policzalne) 😛 ilość czasu (niepoliczalne)
zda RZ ały się nawet rozprawy sądowe dotyczące tego, co znaczy powinien w określonych dokumentach! Językowo powinien wyraża imperatyw trochę innego rodzaju niż musi i według niektórych interpretacji nie jest równie nakazujący. https://sjp.pl/powinien

Kategorycznie nie dopuszczamy etatu ≥ 1. To może Pracownik nie może być zatrudniony na więcej niż jeden etat?
"Nie zdefiniowano X" brzmi w tym przypadku naturalniej niż "X musi być zdefiniowane", ale wtedy tracimy podkreślenie wokół X. Może by je uwydatnić w jakiś sposób wizualny (np. boldem)?

Qwebeck · 2021-03-14T09:16:56Z

Dzięki za szybkie review 👍
Poprawiłem wszystkie komunikaty o których pisałaś,
Co do tego:

"Nie zdefiniowano X" brzmi w tym przypadku naturalniej niż "X musi być zdefiniowane", ale wtedy tracimy podkreślenie wokół X. Może by je uwydatnić w jakiś sposób wizualny (np. boldem)?

to mi się wydaje, że raczej nie ma takiej potrzeby, ponieważ te komunikaty i tak pojawiają się w pod konkretnymi polami, więc i tak wiadomo o co chodzi

Ehevi · 2021-03-14T10:23:54Z

src/components/namestable/worker-edit/worker-name-edit-field.components.tsx

+    {
+      condition: isWorkerWithSameNameExists(),
+      message: `Pracownik o imieniu ${workerName} już istnieje`,
+    },
+    {
+      condition: isWorkerNameEmpty(),
+      message: "Nie zdefiniowano imienia pracownika",
+    },
+  ];
+
+  return (
+    <>
+      <Grid item xs={6}>
+        <Typography className={classes.label}>Imię i nazwisko</Typography>


Właściwie to nie wiem, czy użycie synekdochy (pars pro toto) w tym przypadku jest poprawne. Nie chodzi nam przecież o imię jako samo imię, tylko bardziej o jednoznaczny identyfikator pracownika. To chyba nie jest jakaś niedopuszczalna sytuacja, że w danym miejscu pracują dwie osoby o takich samych imionach.

Masz rację.
Ale nie wiem jak to zamienić: jedyne co przychodzi do głowy - Identyfikator pracownika, ale to brzmi słabo.
Natomiast ta sytuacja o której ty mówisz raczej jest rzadka.
Moim zdaniem lepiej zostać przy tym co jest teraz .
@prenc, co ty o tym myślisz?

Żeby być spójnym można by to nazwać nazwa pracownika (bo zawiera imię i nazwisko, ew. coś jeszcze), ale może to średnio brzmieć.

Co do tego co mówi Agata, to niewiele rozumiem, albo używasz pojęcie imienia wymiennie z nazwą pracownika. Już dopuszczamy kilku pracowników o tym samym imieniu i w teorii, również o tej samej nazwie (case ze zmianą typu umowy).

@Ehevi
Chyba poprawiłem synekdochy xd (jeśli chodziło o to nazwa pola - to Imię i nazwisko, a w komunikacie piszemy tylko o imieniu)

Co do tego, żeby zamienić Imię i nazwisko na coś innego, to tego już nie robiłem.
Myśle, że one i tak to zrozumieją w razie czego.

To może Taki pracownik już istnieje? Skoro zakładamy, że będziemy ich jednoznacznie określać za pomocą imion i nazwisk (co nadal jest synekdochą).

@Ehevi

Co do tego co mówi Agata, to niewiele rozumiem, albo używasz pojęcie imienia wymiennie z nazwą pracownika. Już dopuszczamy kilku pracowników o tym samym imieniu i w teorii, również o tej samej nazwie (case ze zmianą typu umowy).

Ehevi · 2021-03-14T10:26:10Z

te komunikaty i tak pojawiają się w pod konkretnymi polami

To może w takim przypadku wystarczyłoby Należy uzupełnić to pole?

Qwebeck · 2021-03-14T17:08:08Z

te komunikaty i tak pojawiają się w pod konkretnymi polami

To może w takim przypadku wystarczyłoby Należy uzupełnić to pole?

Powiem szczerze, e mi bardziej się podoba aktualna wersja komunikatów.
Ale niech też ktoś inny się wypowie.
@prenc

prenc · 2021-03-14T17:15:43Z

zdażały
@Ehevi
zdarzały? :P

prenc · 2021-03-14T17:33:19Z

te komunikaty i tak pojawiają się w pod konkretnymi polami

To może w takim przypadku wystarczyłoby Należy uzupełnić to pole?

Powiem szczerze, e mi bardziej się podoba aktualna wersja komunikatów.
Ale niech też ktoś inny się wypowie.
@prenc

Byłbym raczej za czymś typu, wybierz typ umowy, wpisz nazwę pracownika, wybierz wymiar etatu

…nput

Qwebeck · 2021-03-14T21:35:59Z

wybierz typ umowy

Ma sens.
Zrobiłem tak mówisz

venglov

Ogólnie super praca, mam tylko kilka drobnych uwag i pytań. Request changes z powodu literówek, wszystkie inne moje wątpliwości do twojej dyspozycji :)

src/components/namestable/use-worker-info.ts

venglov · 2021-03-15T11:55:35Z

...onents/namestable/worker-edit/worker-contract-type-dependent-worknorm-selector.component.tsx

+import { WorkerEmployementContractWorkNormSelector } from "./worker-employement-contract-work-norm-selector.component";
+import { WorkerCivilContractWorkNormSelector } from "./worker-civil-contract-work-norm-selector.component";
+
+export interface WorkerContractTypeDependentWorkTimeSelectorOptions extends FormFieldOptions {


Starałbym się unikać na tyle długich nazw, ponieważ poźniejsze ich wykorzystanie jest słabo czytelne, tym bardziej kiedy jest ich wiele. Może coś w stylu 'SelectorByContractTypeOptions'? Jendak jest to drobna uwaga, a nie request changes :)

Racja
Trochę mnie poniosło xd
Zmieniłem na CombinedWorkNormSelector

venglov · 2021-03-15T11:59:07Z

src/components/namestable/worker-edit/worker-contract-type-selector.component.tsx

+  const classes = useFormFieldStyles();
+
+  const contractOptions = useMemo(
+    () =>


tego pewien nie jestem, ale czy tu nie musi być (): typ?

Słuszna uwaga, dodałem

venglov · 2021-03-15T12:00:29Z

src/components/namestable/worker-edit/worker-edit.component.tsx

+      workerInfo.previousWorkerName = name;
+    }
+  }, [mode, workerInfo, name]);
+  //#region event handlers


komentarze regionów w tym pliku są potrzebne?

Nie są
Nie mniej jednak preferuję żeby one były ponieważ moim zdaniem zwiększają czytelność kodu dzieląc go na logiczne sekcje.

Ponadto to ma taką zaletę, że wszystko zawarte w regionie może być zwinięte

PS
W intellij to chyba wymaga dodatkowej konfiguracji

venglov · 2021-03-15T12:02:37Z

...mponents/namestable/worker-edit/worker-employement-contract-work-norm-selector.component.tsx

+  );
+
+  const contractTimeDropdownOptions = useMemo(
+    () =>


ide proponuje (): { dataCy: string; action: () => void; label: any }[]
nwm czy ma to sens

Mamy zdefiniowany interfejs ButtonData, więc użyłem go

venglov · 2021-03-15T12:27:28Z

src/components/namestable/use-worker-info.ts

+import { ShiftCode } from "../../common-models/shift-info.model";
+import { ContractType, WorkersInfoModel, WorkerType } from "../../common-models/worker-info.model";
+import { ApplicationStateModel } from "../../state/models/application-state.model";
+import { WorkerInfoExtendedInterface } from "./worker-edit/worker-edit.models";


może być krótszy

import { WorkerInfoExtendedInterface } from "./worker-edit";

src/components/namestable/worker-edit/worker-civil-contract-work-norm-selector.component.tsx

...onents/namestable/worker-edit/worker-contract-type-dependent-worknorm-selector.component.tsx

venglov · 2021-03-15T12:32:12Z

src/components/namestable/worker-edit/worker-name-edit-field.components.tsx

+  );
+
+  const isWorkerWithSameNameExists = useCallback((): boolean => {
+    const isWorkerNameInvalid =


myślę że ta zmienna nie jest potrzebna

Preferuję wynosić wyniki operacji do zmiennych nawet jeśli ta zmienna będzie w następnej linii zwrócona w returnie.
Główna zaleta tego - to łatwiejsze debugowanie, bo czasami chcesz zobaczyć co zwraca funkcja będą w tej funkcji i wtedy taka zmienna na to pozwala.

src/components/namestable/worker-edit/worker-position-selector.component.tsx

Ehevi · 2021-03-16T05:49:44Z

src/components/namestable/worker-edit/worker-civil-contract-work-norm-selector.component.tsx

+        <Typography className={classes.label}>Ilość godzin</Typography>
+        <TextField
+          fullWidth
+          name="civilTime"
+          data-cy="input-civil-time"
+          value={convertToNormalHours(employementTime)}
+          type="number"
+          style={{
+            width: 100,
+          }}
+          className={classes.formInput}
+          onBlur={(event): void => setWorkerTime(toWorkerNorm(event.target.value))}
+          color="primary"
+        />


Ta wartość nie powinna być wprowadzana przez użytkownika? Bo u mnie nie to działa, strzałki góra-dół też nie, cały czas jest 160.

No, i policzalne godziny ;)

u mnie też nie działa

Dzięki za uwagę.
Poprawiłem, teraz działa dobrze

prenc

Sorry, ale nie podoba mi się to, że otwierasz drawer i od razu wszystko na czerwono się świeci. Raczej powinno być tak, że przycisk do zapisywania pracownika jest cały czas aktywny, a przy jego naciśnięciu pola są sprawdzane i jak coś jest nie tak to wyświetla się czerwona informacja.

Reszta dla mnie git. + to co Agata już powiedziała

Qwebeck

Sorry, ale nie podoba mi się to, że otwierasz drawer i od razu wszystko na czerwono się świeci. Raczej powinno być tak, że przycisk do zapisywania pracownika jest cały czas aktywny, a przy jego naciśnięciu pola są sprawdzane i jak coś jest nie tak to wyświetla się czerwona informacja.

Zrobiłem tak, żeby komunikaty pojawiały się nie wcześniej niż po wprowadzeniu jakiejś informacji do odpowiednich pół

…nput

prenc

UI się rozwala przy wpisywaniu etatu bardzo łatwo

Ogólnie jakieś dziwnie duże przerwy są pomiędzy kolejnymi elementami.

Qwebeck · 2021-03-18T18:05:27Z

Dodałem scroll który pojawia się w momentach kiedy zoom jest za duży

…nput

* TASK-338 Enable database clear when error appears (#249) * Save file with proper name, save all db docs when error button clicked * Modify database export and export all schedules in zip * Set proper zip name * Fix and extend filehelper test * Move handle dummp to file helper, remove comments * Fix file helper test * Add custom types instead of inline dicts * Save correct timestamp * TASK-343 Handle rendering crushes at schedule level (#257) * TASK-323 Add min char number indication in report issue modal (#262) * Task 348 Enable undo on corrupted schedule page (#265) * TASK-345 Improve descriptions of found schedule's issues (#263) * TASK-371 Bug: wrong errors when 31-day month (#271) * export workers (#267) Co-authored-by: Tomasz Pęcak <[email protected]> Co-authored-by: Paweł Renc <[email protected]> * TASK-324 Reduce hour number based on previously scheduled shifts for L4 (#259) * TASK-319 Export number of daytime employees information (#260) * Task 291 Copy month to full schedule (including next month) (#272) * Fixes bad key setting (#275) * TASK-322 Bug: fix missing colors in Excel export for employees having no shifts (#258) * TASK-333 && TASK-335 Asking about unsaved changes when exiting edit mode && disabling undo/redo/save buttons (#270) * TASK-370 Fix restoring state on corrupted schedule page (#276) * TASK-362 Parse workers info from excel (#269) * TASK-375 Bug: app crashes when copying from month ending on sunday (#280) * TASK-314 Another iteration over excel layout of exported schedule (#278) * bugfixes * implements function to decide font color * added links * small fixes Co-authored-by: Tomasz Pęcak <[email protected]> * TASK-389 Close issue drawer when exiting edit mode (#285) * Bug fixed. Cypress passing. * Added isEditMode watcher for auto drawer closing. * TASK-376 Error modal doesn't appear on app error (#286) * Handle error on higher tree level * Add error message for testing purposes * TASK-350 Keep shifts scheduled in next month's days (#283) * Add corrupted info to schedule state * Stash branch changes * Restore prev storage initialization * Do not replace free shifts in first week * TASK-387 Sort shifts in dropdown (#282) * TASK-361 autocomplete component rendering fix (#284) * TASK-304 Another iteration over drawer for managing workers (#279) * TASK-388 Now edit state is properly false after leaving edit mode. BUGFIXED (#290) * TASK-256 Add exporting employees' small calendar to pdf (#277) * TASK-379 Add worker to single month (#289) * TASK-339 & TASK-382 fix all console errors & increase folding performance (#288) * TASK-391 fixed calculation of worker norm when added during the month with parttime contract. (#295) * TASK-390 Stick footer to bottom of page, not viewport (#294) * Change footer to be the end of the content, rather than sticked to the bottom of the screen. Remove electron block * Footer always on the bottom of the page * Repair jira like drawer after footer changes. Repair zarzadzanie tab Co-authored-by: Tomasz Pęcak <[email protected]> Co-authored-by: Ehevi <[email protected]> Co-authored-by: Paulina <[email protected]> Co-authored-by: Bohdan <[email protected]> Co-authored-by: Vyacheslav Trushkov <[email protected]> Co-authored-by: kopcion <[email protected]> Co-authored-by: Jan Ślażyński <[email protected]> Co-authored-by: Maciej Bielech <[email protected]>

TASK-304 usable worker drawer

3f40589

Qwebeck requested review from Ehevi, venglov and mbielech March 13, 2021 23:59

Merge branch 'develop' into TASK-304-work-hour-conversion-in-worker-i…

fbb8990

…nput

Qwebeck commented Mar 14, 2021

View reviewed changes

Ehevi requested changes Mar 14, 2021

View reviewed changes

Qwebeck requested review from prenc and Ehevi March 14, 2021 09:17

Ehevi reviewed Mar 14, 2021

View reviewed changes

prenc changed the title ~~TASK-304 usable worker drawer~~ TASK-304 Another iteration over drawer for managing workers Mar 14, 2021

Fixes validation messages

2dac106

Qwebeck force-pushed the TASK-304-work-hour-conversion-in-worker-input branch from 4e657d4 to 2dac106 Compare March 14, 2021 21:35

Merge branch 'develop' into TASK-304-work-hour-conversion-in-worker-i…

860fbb1

…nput

Qwebeck requested a review from Ehevi March 14, 2021 21:38

venglov requested changes Mar 15, 2021

View reviewed changes

Ehevi reviewed Mar 16, 2021

View reviewed changes

prenc requested changes Mar 16, 2021

View reviewed changes

Qwebeck removed the request for review from mbielech March 17, 2021 20:07

Adds normal validation meesages, addresses review comments

dcff596

Qwebeck commented Mar 17, 2021

View reviewed changes

Merge branch 'develop' into TASK-304-work-hour-conversion-in-worker-i…

30dff1c

…nput

Qwebeck requested a review from prenc March 17, 2021 21:46

Qwebeck requested review from Ehevi and venglov March 17, 2021 21:46

prenc requested changes Mar 18, 2021

View reviewed changes

Adds scroll to drawer

b82c730

Qwebeck force-pushed the TASK-304-work-hour-conversion-in-worker-input branch from af33ad5 to b82c730 Compare March 18, 2021 18:00

Qwebeck requested a review from prenc March 18, 2021 18:05

Merge branch 'develop' into TASK-304-work-hour-conversion-in-worker-i…

7f56a2d

…nput

Qwebeck merged commit 401b6eb into develop Mar 20, 2021

Qwebeck deleted the TASK-304-work-hour-conversion-in-worker-input branch March 20, 2021 10:28

prenc pushed a commit that referenced this pull request Mar 22, 2021

TASK-304 Another iteration over drawer for managing workers (#279)

7d8d721

TASK-304 Another iteration over drawer for managing workers #279

TASK-304 Another iteration over drawer for managing workers #279

Conversation

Qwebeck commented Mar 13, 2021

cypress bot commented Mar 14, 2021 • edited Loading

Test summary

Run details

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ehevi left a comment • edited Loading

Choose a reason for hiding this comment

Qwebeck commented Mar 14, 2021

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

Ehevi commented Mar 14, 2021

Qwebeck commented Mar 14, 2021

prenc commented Mar 14, 2021

prenc commented Mar 14, 2021

Qwebeck commented Mar 14, 2021

venglov 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

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

prenc left a comment • edited Loading

Choose a reason for hiding this comment

Qwebeck left a comment

Choose a reason for hiding this comment

prenc left a comment

Choose a reason for hiding this comment

Qwebeck commented Mar 18, 2021

cypress bot commented Mar 14, 2021 •

edited

Loading

Ehevi left a comment •

edited

Loading

prenc left a comment •

edited

Loading