WEB-604: Fix partial-day cashier time handling#3009
WEB-604: Fix partial-day cashier time handling#3009AnvayKharb wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Create Cashier Component src/app/organization/tellers/cashiers/create-cashier/create-cashier.component.html, src/app/organization/tellers/cashiers/create-cashier/create-cashier.component.ts |
Template now conditionally renders start/end hour & minute selectors when isFullDay is false. Component adds hours and minutes arrays, initializes time controls, validates time range, and builds an explicit payload that includes time fields only when applicable. |
Edit Cashier Component src/app/organization/tellers/cashiers/edit-cashier/edit-cashier.component.html, src/app/organization/tellers/cashiers/edit-cashier/edit-cashier.component.ts |
Template conditionally shows hour/min selectors; component parses existing startTime/endTime into hour/min controls, exposes hours/minutes, validates time range, and appends time fields to payload only when not full-day. |
Translations (multiple locales) src/assets/translations/.../*.json (examples: en-US.json, de-DE.json, fr-FR.json, es-CL.json, es-MX.json, it-IT.json, ko-KO.json, lt-LT.json, lv-LV.json, ne-NE.json, pt-PT.json, sw-SW.json, cs-CS.json) |
Added Hour and Minute translation keys in each locale JSON. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant UI as Create/Edit Component
participant Service as CashierService
participant API as Backend API
User->>UI: open form / toggle "Full Day"
UI->>User: render date fields and conditional time selectors
User->>UI: submit form
alt isFullDay == true
UI->>Service: submit payload (no time fields)
else isFullDay == false
UI->>UI: validate time range (hour/min)
UI->>Service: submit payload (includes hour/min and derived startTime/endTime)
end
Service->>API: POST create/edit cashier
API-->>Service: response
Service-->>UI: result
UI-->>User: show success/error
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
- alberto-art3ch
- IOhacker
- gkbishnoi07
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly identifies the main change: fixing partial-day cashier time handling, which aligns with the PR's core objective of supporting time selection when Full Day is unchecked. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@src/app/organization/tellers/cashiers/create-cashier/create-cashier.component.ts`:
- Around line 126-139: The submit() flow in CreateCashierComponent currently
copies start/end hour/min fields without validating that the partial-day end
time is strictly after the start time; add a guard in submit() right after you
read createCashierFormData.hourStartTime/minStartTime and hourEndTime/minEndTime
to convert those to a comparable value (e.g., total minutes = hour*60 + min) and
if endMinutes <= startMinutes stop submission and surface a validation error
(set a form control error, show the existing alert/toast, or return early) so
data.startTime/data.endTime are never submitted with an invalid range; reference
createCashierFormData, hourStartTime, minStartTime, hourEndTime, minEndTime,
data.startTime/data.endTime and the submit() handler when making the change.
In
`@src/app/organization/tellers/cashiers/edit-cashier/edit-cashier.component.html`:
- Around line 101-147: The form currently has no cross-field validation so
editCashierForm can save an end time earlier than the start time; add a custom
validator on the FormGroup (attached when creating editCashierForm in the
component class) that reads hourStartTime/minStartTime and
hourEndTime/minEndTime, converts them to total minutes (e.g., startMinutes =
hourStartTime*60 + minStartTime) and returns { invalidTimeRange: true } when
endMinutes <= startMinutes, otherwise null; ensure the validator is registered
on editCashierForm (or via setValidators) and update the submit handler (e.g.,
the component's submit/save method) to call
editCashierForm.updateValueAndValidity(), markAllAsTouched() and prevent submit
when editCashierForm.invalid so the invalidTimeRange error blocks saving.
In
`@src/app/organization/tellers/cashiers/edit-cashier/edit-cashier.component.ts`:
- Around line 84-94: Guard time parsing in edit-cashier.component.ts by ensuring
startTime/endTime split always yields two parts: when extracting
hourStart/minStart and hourEnd/minEnd from startTime and endTime, provide
defaults during destructuring (e.g., const [hourStart='09', minStart='00'] =
(startTime || '09:00').split(':')) or validate for ':' before splitting so
minStart/minEnd never become undefined and subsequent padStart calls cannot
throw; update the parsing around startTime, endTime, hourStart, minStart,
hourEnd, minEnd to use these defaults or a fallback time string.
- Around line 144-157: When preparing the payload in submit() of
EditCashierComponent, add a guard that parses
editCashierFormData.hourStartTime/minStartTime and hourEndTime/minEndTime into
integers and compares the resulting start and end minutes (or timestamps) to
ensure end > start; if the check fails, set a form-level error or show a
validation message and return early to prevent constructing
data.hourStartTime/minStartTime/hourEndTime/minEndTime and
data.startTime/data.endTime or sending the request. Use the existing variables
(editCashierFormData, hourStartTime, minStartTime, hourEndTime, minEndTime,
data.startTime, data.endTime) to implement the check and short-circuit the
submit flow on invalid ranges.
|
@shubhamkumar9199 could you please give us your feedback? |
|
@IOhacker @AnvayKharb The "Full Day?" checkbox should be placed at the bottom of the date fields to create a more balanced layout and avoid visual unevenness. |
|
Hi @IOhacker and @shubhamkumar9199 thanks for your review and i have made changes accordingly and please review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/app/organization/tellers/cashiers/create-cashier/create-cashier.component.html`:
- Around line 86-132: The partial-day time selects (hourStartTime, minStartTime,
hourEndTime, minEndTime) lack required and cross-field validation when
createCashierForm.value.isFullDay is false; update the component to add/remove
Validators.required on those controls whenever isFullDay changes (use
createCashierForm.get('isFullDay').valueChanges to call setValidators and
updateValueAndValidity), implement a custom cross-field validator (e.g.,
validateTimeRange or timeRangeValidator) attached to the form/group that
computes start and end minutes from hourStartTime/minStartTime and
hourEndTime/minEndTime and returns an error when end <= start, and update the
template to show mat-error messages under each mat-form-field and a general
error message for the timeRange validation so users see required and range
errors before submit.
7a46c04 to
8a6ddbb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/app/organization/tellers/cashiers/edit-cashier/edit-cashier.component.ts`:
- Around line 145-152: Move the time-range check out of the submit handler and
into a form-level validator: add a private timeRangeValidator function on the
component that returns { invalidTimeRange: true } when isFullDay is false and
the computed startMinutes/endMinutes are NaN or endMinutes <= startMinutes (use
the group controls hourStartTime, minStartTime, hourEndTime, minEndTime and
respect isFullDay), attach this validator to editCashierForm when the form group
is created, remove the this.editCashierForm.setErrors({ invalidTimeRange: true
}) call from the submit method, and at the start of submit call
this.editCashierForm.updateValueAndValidity() then if
(this.editCashierForm.invalid) { this.editCashierForm.markAllAsTouched();
return; } so the error clears automatically when values change.
♻️ Duplicate comments (1)
src/app/organization/tellers/cashiers/create-cashier/create-cashier.component.ts (1)
133-140: Form-level invalidTimeRange persists after fixing time or toggling Full Day, disabling the submit button.When the time range validation fails at line 138,
setErrors({ invalidTimeRange: true })is called on the form. However, there is no mechanism to clear this error when the user modifies the time fields or toggles isFullDay. The error only clears on the nextsubmit()call, leaving the form invalid and the submit button disabled until resubmission.Move the validation to a form-level validator so it recomputes automatically on value changes:
Suggested fix: group-level validator + submit guard
setCreateCashierForm() { - this.createCashierForm = this.formBuilder.group({ + this.createCashierForm = this.formBuilder.group( + { staffId: ['', Validators.required], description: [''], startDate: ['', Validators.required], endDate: ['', Validators.required], isFullDay: [true], hourStartTime: ['09'], minStartTime: ['00'], hourEndTime: ['17'], minEndTime: ['00'] - }); + }, + { validators: this.timeRangeValidator } + ); } submit() { const createCashierFormData = this.createCashierForm.value; const locale = this.settingsService.language.code; const dateFormat = this.settingsService.dateFormat; const prevStartDate: Date = this.createCashierForm.value.startDate; const prevEndDate: Date = this.createCashierForm.value.endDate; if (createCashierFormData.startDate instanceof Date) { createCashierFormData.startDate = this.dateUtils.formatDate(prevStartDate, dateFormat); } if (createCashierFormData.endDate instanceof Date) { createCashierFormData.endDate = this.dateUtils.formatDate(prevEndDate, dateFormat); } const data: any = { staffId: createCashierFormData.staffId, description: createCashierFormData.description, startDate: createCashierFormData.startDate, endDate: createCashierFormData.endDate, isFullDay: createCashierFormData.isFullDay, dateFormat, locale }; + this.createCashierForm.updateValueAndValidity(); + if (this.createCashierForm.invalid) { + this.createCashierForm.markAllAsTouched(); + return; + } if (!createCashierFormData.isFullDay) { const hourStart = createCashierFormData.hourStartTime; const minStart = createCashierFormData.minStartTime; const hourEnd = createCashierFormData.hourEndTime; const minEnd = createCashierFormData.minEndTime; - const startMinutes = Number(hourStart) * 60 + Number(minStart); - const endMinutes = Number(hourEnd) * 60 + Number(minEnd); - - if (Number.isNaN(startMinutes) || Number.isNaN(endMinutes) || endMinutes <= startMinutes) { - this.createCashierForm.setErrors({ invalidTimeRange: true }); - return; - } data.hourStartTime = hourStart; data.minStartTime = minStart; data.hourEndTime = hourEnd; data.minEndTime = minEnd; data.startTime = `${hourStart}:${minStart}`; data.endTime = `${hourEnd}:${minEnd}`; } this.organizationService.createCashier(this.cashierTemplate.tellerId, data).subscribe((response: any) => { this.router.navigate(['../'], { relativeTo: this.route }); }); }private timeRangeValidator = (group: UntypedFormGroup) => { if (group.get('isFullDay')?.value) { return null; } const hourStart = group.get('hourStartTime')?.value; const minStart = group.get('minStartTime')?.value; const hourEnd = group.get('hourEndTime')?.value; const minEnd = group.get('minEndTime')?.value; const startMinutes = Number(hourStart) * 60 + Number(minStart); const endMinutes = Number(hourEnd) * 60 + Number(minEnd); if (Number.isNaN(startMinutes) || Number.isNaN(endMinutes) || endMinutes <= startMinutes) { return { invalidTimeRange: true }; } return null; };
cce1f77 to
3d38df0
Compare
alberto-art3ch
left a comment
There was a problem hiding this comment.
Please see my comments
3d38df0 to
d9ae2ce
Compare
|
@IOhacker @alberto-art3ch sir please review |
d9ae2ce to
554740d
Compare
1538b2e to
e358842
Compare
|
@IOhacker changed the time to default 00 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/app/organization/tellers/cashiers/create-cashier/create-cashier.component.ts`:
- Around line 116-148: The time-range validation currently calls
this.createCashierForm.setErrors(...) in the submit flow (using
createCashierFormData and the manual startMinutes/endMinutes checks) which
leaves a persistent form-level error; instead implement a form-level validator
function (e.g., validateCashierTimeRange) that reads
hourStartTime/minStartTime/hourEndTime/minEndTime from the FormGroup and returns
{ invalidTimeRange: true } when end <= start or null otherwise, attach it to the
FormGroup creation for createCashierForm, remove the manual validation block in
the submit handler (the Number.isNaN and setErrors logic) and simply rely on
this.createCashierForm.valid before building the data and persisting, and keep
the code that adds time fields to the data when createCashierFormData.isFullDay
is false.
♻️ Duplicate comments (1)
src/app/organization/tellers/cashiers/edit-cashier/edit-cashier.component.ts (1)
155-169: Consider form-level time-range validation so errors clear after edits.Using
setErrors()insidesubmit()can keepinvalidTimeRangestuck even after the user fixes the values. A FormGroup validator will recompute on changes and clear automatically.
|
@AnvayKharb it is possible to add the screenshot or a video when the fields are hidden? |
|
@IOhacker yes i have now modified the description accordingly |
|
LGTM |
|
@IOhacker rhanks for the review |
This PR fixes the Create Cashier flow to support partial-day cashiers by correctly handling time (hours and minutes) when Full Day is unchecked. It ensures the expected payload is sent and uses i18n for all new UI labels.
before
after
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.