Bugfix/table cell date and address fix #3930
Conversation
📝 WalkthroughWalkthroughAddress mapping switched to Ukrainian-specific fields; date-cell template, styles, locale input and logic refactored with expanded tests; order flows now prefetch existing-order info and may cancel payments; added GetExistingOrderDetailsFail action/effect that shows a snack and navigates on 403; validators, i18n, and snackbar config updated. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Component/UI
participant Store as NgRx Store
participant Effect as OrderEffects
participant API as OrderService/API
participant Snack as MatSnackBarService
participant Router as Router
UI->>Store: dispatch GetExistingOrderDetails(orderId)
Store->>Effect: effect handles GetExistingOrderDetails
Effect->>API: fetch existing order details
alt success
API-->>Effect: order details
Effect-->>Store: dispatch GetExistingOrderDetailsSuccess
else 403
API-->>Effect: HttpErrorResponse(403)
Effect-->>Store: dispatch GetExistingOrderDetailsFail(error)
Effect->>Snack: show 'order-not-authorized' snack
Effect->>Router: navigate('/ubs/user/orders')
else other error
API-->>Effect: HttpErrorResponse(error)
Effect-->>Store: dispatch GetExistingOrderDetailsFail(error)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.ts (1)
92-96: Consider using a proper logging service instead ofconsole.error.Direct console logging may be stripped in production builds and isn't configurable. If this is intentional error handling for debugging, consider using the application's logging service for consistency and observability.
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts (1)
22-25: Consider resetting mock spies between tests and verifyingpicker.open()is called.The
mockDatepickerspies are never reset between tests andopen()is never verified. Since theedit()method callspicker.open()when orders aren't blocked, add assertions to verify this behavior.🔎 Proposed addition
// In beforeEach, reset the spies: beforeEach(() => { (mockDatepicker.open as jasmine.Spy).calls.reset(); (mockDatepicker.close as jasmine.Spy).calls.reset(); // ... rest of setup }); // Add a test to verify picker opens when not blocked: it('should open picker when blockOrders returns undefined', () => { const service = TestBed.inject(AdminTableService); spyOn(service, 'blockOrders').and.returnValue(of([undefined])); component.isAllChecked = false; component.ordersToChange = []; component.id = 1; component.edit(mockDatepicker as MatDatepicker<Date>); expect(mockDatepicker.open).toHaveBeenCalled(); });src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.ts (1)
128-130: EmitaddressChangedevent after successful address update.The newly added
addressChangedoutput (line 32) should likely be emitted here after the address is successfully updated. Currently, the subscription only unblocks orders but doesn't notify parent components of the change.🔎 Emit the addressChanged event
.subscribe(() => { this.adminTableService.unblockOrders([this.id]).subscribe(); + this.addressChanged.emit(this.id); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/store/functions.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.htmlsrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.scsssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/ubs-admin-table.component.html
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-01T22:32:58.734Z
Learnt from: AndrewKozovyi
Repo: ita-social-projects/GreenCityClient PR: 3770
File: src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts:103-110
Timestamp: 2025-08-01T22:32:58.734Z
Learning: In UbsAdminCustomersComponent (src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts), using take(1) instead of takeUntilDestroyed(this.destroyRef) for the adminTableOfCustomersSelector$ subscription prevents excessive additional functionality from being triggered and works correctly for the component's initialization sequence.
Applied to files:
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.ts
📚 Learning: 2025-12-11T20:44:21.136Z
Learnt from: witolDark
Repo: ita-social-projects/GreenCityClient PR: 3921
File: src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts:214-214
Timestamp: 2025-12-11T20:44:21.136Z
Learning: In Angular projects using HttpClient, HTTP call observables complete after emitting a single value. Therefore, take(1) or takeUntil is not strictly required to prevent memory leaks for typical single-emission requests. However, using take(1) (or takeUntil in combination with a component-destruction signal) can improve readability and guard against accidental multiple emissions if the observable is reused or transformed. For files like src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts and other services using HttpClient, rely on HttpClient completion for single requests, but consider adding take(1) when subscribing in long-lived contexts or when the observable could emit more than once due to retries or stream augmentation.
Applied to files:
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.tssrc/app/store/functions.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.ts
🧬 Code graph analysis (2)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.ts (1)
src/app/ubs/ubs-admin/models/ubs-admin.interface.ts (1)
IColumnBelonging(501-506)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.ts (1)
src/app/store/actions/ubs-admin.actions.ts (1)
SetCursorWaite(10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (18.x)
🔇 Additional comments (9)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.ts (2)
49-58: LGTM!The
langsetter properly propagates locale changes to theDateAdapter, and the constructor injection is correctly typed.
60-63: LGTM!The early return guards properly handle uneditable status and filter keyboard events to only respond to Enter/Space for accessibility compliance.
src/app/ubs/ubs-admin/components/ubs-admin-table/ubs-admin-table.component.html (1)
362-373: LGTM!The
[lang]="currentLang"binding is correctly added to the date cell component, consistent with other cell components in the table that also receive the language binding.src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.html (2)
1-10: LGTM!The form field structure with disabled state binding, min date constraint, and dateChange event handler is well-implemented.
11-16: The toggle's click handler is necessary and correct. Themat-datepicker-toggleonly has built-in click handling when the[for]attribute is present. This toggle intentionally omits[for]and uses custom(click)and(keydown)handlers to implement the blocking validation logic viablockOrders()before opening the picker. No double-open occurs since the toggle has no default behavior to conflict with theedit()method.Likely an incorrect or invalid review comment.
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts (1)
101-120: LGTM!The test correctly constructs the expected date format to match the component's
changeData()implementation, ensuring the assertion validates the actual behavior.src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.scss (2)
19-43: LGTM!The host-scoped styles properly encapsulate Material form field customizations within this component, preventing style leakage to other parts of the application.
45-65: Note:::ng-deepis deprecated but currently necessary.The use of
:host ::ng-deepis the correct pattern for scoping Material component overrides. Be aware that::ng-deepis deprecated, though Angular has not provided a replacement for this use case yet.src/app/store/functions.ts (1)
1-12: [Rewritten review comment]
[Classification tag]
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.ts
Show resolved
Hide resolved
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.ts
Show resolved
Hide resolved
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.ts
Outdated
Show resolved
Hide resolved
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.ts
Outdated
Show resolved
Hide resolved
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.ts
Outdated
Show resolved
Hide resolved
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/app/store/functions.ts (1)
1-1: Theanytype issue remains unaddressed.The function parameter is still typed as
any, which was flagged in a previous review. Please define a proper interface for type safety as suggested in the earlier review.
🧹 Nitpick comments (1)
src/app/store/functions.ts (1)
3-8: Add type safety to the function parameter instead ofany.The function parameter is typed as
any, which lacks type safety. While the interfaceIShortAddress.orderAddressExportDetailsdefines all address fields (region, city, district, street, houseNumber, entranceNumber) as required strings, usinganybypasses TypeScript's type checking and could allow incomplete data to be passed.Either type the
inputparameter asIOrderAddressExportDetails(or extract/export the nested interface) or add consistent fallbacks across all fields for defensive robustness. Currently, onlyhouseCorpusandaddressCommenthave fallbacks (|| ''), creating inconsistent handling.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/store/functions.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-input/table-cell-input.component.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-11T20:44:21.136Z
Learnt from: witolDark
Repo: ita-social-projects/GreenCityClient PR: 3921
File: src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts:214-214
Timestamp: 2025-12-11T20:44:21.136Z
Learning: In Angular projects using HttpClient, HTTP call observables complete after emitting a single value. Therefore, take(1) or takeUntil is not strictly required to prevent memory leaks for typical single-emission requests. However, using take(1) (or takeUntil in combination with a component-destruction signal) can improve readability and guard against accidental multiple emissions if the observable is reused or transformed. For files like src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts and other services using HttpClient, rely on HttpClient completion for single requests, but consider adding take(1) when subscribing in long-lived contexts or when the observable could emit more than once due to retries or stream augmentation.
Applied to files:
src/app/store/functions.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (18.x)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts (1)
28-31: Reset mock datepicker spies between tests to prevent false positives.The
mockDatepickerspies are created once at module level but never reset. Spy call counts accumulate across tests, which can cause false positives when checkingtoHaveBeenCalled()ortoHaveBeenCalledTimes().Proposed fix: reset spies in beforeEach
beforeEach(() => { jasmine.clock().install(); jasmine.clock().mockDate(new Date()); fixture = TestBed.createComponent(TableCellDateComponent); component = fixture.componentInstance; adminTableService = TestBed.inject(AdminTableService); store = TestBed.inject(MockStore); cdr = fixture.componentRef.injector.get(ChangeDetectorRef); + + (mockDatepicker.open as jasmine.Spy).calls.reset(); + (mockDatepicker.close as jasmine.Spy).calls.reset(); fixture.detectChanges(); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-01T22:32:58.734Z
Learnt from: AndrewKozovyi
Repo: ita-social-projects/GreenCityClient PR: 3770
File: src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts:103-110
Timestamp: 2025-08-01T22:32:58.734Z
Learning: In UbsAdminCustomersComponent (src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts), using take(1) instead of takeUntilDestroyed(this.destroyRef) for the adminTableOfCustomersSelector$ subscription prevents excessive additional functionality from being triggered and works correctly for the component's initialization sequence.
Applied to files:
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
📚 Learning: 2025-12-11T20:44:21.136Z
Learnt from: witolDark
Repo: ita-social-projects/GreenCityClient PR: 3921
File: src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts:214-214
Timestamp: 2025-12-11T20:44:21.136Z
Learning: In Angular projects using HttpClient, HTTP call observables complete after emitting a single value. Therefore, take(1) or takeUntil is not strictly required to prevent memory leaks for typical single-emission requests. However, using take(1) (or takeUntil in combination with a component-destruction signal) can improve readability and guard against accidental multiple emissions if the observable is reused or transformed. For files like src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts and other services using HttpClient, rely on HttpClient completion for single requests, but consider adding take(1) when subscribing in long-lived contexts or when the observable could emit more than once due to retries or stream augmentation.
Applied to files:
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
🧬 Code graph analysis (1)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts (1)
src/app/store/actions/ubs-admin.actions.ts (1)
SetCursorWaite(10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (18.x)
🔇 Additional comments (4)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts (4)
214-312: Good test coverage for date formatting and edge cases.The
changeData()tests comprehensively cover:
- Standard date formatting with leading zeros
- Boundary conditions (year start/end)
- Invalid input handling (null/undefined)
- Type coercion from string to Date
314-358: LGTM!Input property tests use the modern
setInput()API and provide good coverage for all relevant inputs.
360-397: Integration tests correctly validate end-to-end flows.The tests properly verify the complete edit workflow including cursor state management, blocking check, and conditional datepicker opening. Line 394 correctly expects the datepicker to NOT open when orders are blocked.
58-72: LGTM!Initialization tests appropriately verify component creation and default state.
...p/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
Show resolved
Hide resolved
…icker utc date formatting, styles and locale
witolDark
force-pushed
the
Spelina/style-leakage-fix
branch
from
e002205 to
e247f11
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.ts (2)
132-164: Missing error handling and unsubscription for the observable chain.The refactored
editOrPayPopupmethod lacks:
- Error handling - if
getExistingOrderInfofails, the user receives no feedbacktakeUntil(this.destroy$)- potential subscription leak if component is destroyed while request is in flight🔎 Proposed fix
editOrPayPopup(order: IUserOrderInfo) { this.orderService .getExistingOrderInfo(order.id) .pipe( + takeUntil(this.destroy$), switchMap((info) => this.dialog .open(DialogPopUpComponent, { data: this.editOrPayDialogData, autoFocus: true }) .afterClosed() .pipe(map((res) => ({ res, info }))) ) ) - .subscribe(({ res, info }) => { + .subscribe({ + next: ({ res, info }) => { if (res) { if (info.paymentLink) { window.location.href = info.paymentLink; } else { this.openOrderPaymentPopUp(info); } return; } if (res === false) { if (this.isOrderHalfPaid(info)) { this.openOrderPaymentPopUp(info); } else { this.getDataForLocalStorage(info); } } - }); + }, + error: () => this.snackBarService.openSnackBar('error') + }); }
217-228: Bug:hasPaymentLinkis never set onorderDetails, causing dead code.The condition on line 247 checks
this.orderDetails.hasPaymentLink, but this property is never assigned whenorderDetailsis constructed in lines 217-228. ThecancelExistingPaymentlogic will never execute.🔎 Proposed fix - set hasPaymentLink from the order info
this.orderDetails = { additionalOrders: order.additionalOrders, bags: this.bags, certificates: [], certificatesSum: 0, finalSum: order.orderFullPrice, orderComment: order.orderComment, points: this.bonuses, pointsSum: 0, pointsToUse: 0, - total: order.orderFullPrice + total: order.orderFullPrice, + hasPaymentLink: !!order.paymentLink };Also applies to: 247-247
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.tssrc/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-01T22:32:58.734Z
Learnt from: AndrewKozovyi
Repo: ita-social-projects/GreenCityClient PR: 3770
File: src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts:103-110
Timestamp: 2025-08-01T22:32:58.734Z
Learning: In UbsAdminCustomersComponent (src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts), using take(1) instead of takeUntilDestroyed(this.destroyRef) for the adminTableOfCustomersSelector$ subscription prevents excessive additional functionality from being triggered and works correctly for the component's initialization sequence.
Applied to files:
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.tssrc/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.ts
📚 Learning: 2025-12-11T20:44:21.136Z
Learnt from: witolDark
Repo: ita-social-projects/GreenCityClient PR: 3921
File: src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts:214-214
Timestamp: 2025-12-11T20:44:21.136Z
Learning: In Angular projects using HttpClient, HTTP call observables complete after emitting a single value. Therefore, take(1) or takeUntil is not strictly required to prevent memory leaks for typical single-emission requests. However, using take(1) (or takeUntil in combination with a component-destruction signal) can improve readability and guard against accidental multiple emissions if the observable is reused or transformed. For files like src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts and other services using HttpClient, rely on HttpClient completion for single requests, but consider adding take(1) when subscribing in long-lived contexts or when the observable could emit more than once due to retries or stream augmentation.
Applied to files:
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.tssrc/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (18.x)
🔇 Additional comments (2)
src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.ts (1)
119-139: Nestediiflogic for conditional payment cancellation looks correct.The implementation correctly:
- Evaluates
existingOrderId >= 0to determine if this is an existing order- Conditionally cancels the existing payment when
hasPaymentLinkis true before processing- Chains the cancellation with
switchMapto ensure sequential execution- Handles errors by redirecting to the confirm page via
finalizeOne minor consideration: the subscription lacks
takeUntil(this.$destroy), but since the observable chain completes after a single emission (HTTP calls) and the component redirects on completion, this is acceptable per the project's conventions noted in the learnings.src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.ts (1)
146-162: No type compatibility issues exist.
getExistingOrderInforeturnsObservable<IUserOrderInfo>, and all three methods (openOrderPaymentPopUp,isOrderHalfPaid,getDataForLocalStorage) are explicitly typed to acceptIUserOrderInfo. The interface contains all required properties (id,amountBeforePayment,paymentLink,paymentStatusEn,additionalOrders,orderFullPrice, etc.). TypeScript's type system ensures complete compatibility at compile time.Likely an incorrect or invalid review comment.
src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.ts
Outdated
Show resolved
Hide resolved
Made kyiv city invalid for kyiv region validator fixed existring order query params handling for order that doesnt belong to the user
There was a problem hiding this comment.
Actionable comments posted: 4
Fix all issues with AI Agents 🤖
In @src/app/store/effects/order.effects.ts:
- Line 71: Add a reducer handler for the GetExistingOrderDetailsFail action so
isOrderDetailsLoading is set back to false and the error payload is stored (or
cleared) for consumers; locate the reducer using the createReducer handling
GetExistingOrderDetails and GetExistingOrderDetailsSuccess and add a case for
GetExistingOrderDetailsFail that sets isOrderDetailsLoading: false and copies
action.error (or sets an errors field) so components relying on the
isOrderDetailsLoading selector won’t remain stuck on error.
In @src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.ts:
- Line 73: The flag changedTariffForExistingOrder is defined but never set to
true, so remove the dead code: delete the private property declaration
changedTariffForExistingOrder and remove the conditional block that checks it
(the if (...) { ... } block that relies on changedTariffForExistingOrder);
search for any other references to changedTariffForExistingOrder in
ubs-order-details component and remove or refactor them as needed, then run the
TypeScript build/tests to ensure no lingering references remain.
In
@src/app/ubs/ubs/components/ubs-order-details/ubs-order-location-popup/ubs-order-location-popup.component.ts:
- Around line 81-84: In saveLocation(), the undefined identifier tariff is used
and will throw at runtime; replace tariff.id with this.selectedTariff.id and
guard against missing selection (e.g., if (!this.selectedTariff) return or
handle error) before calling this.localStorageService.setTariffId and closing
the dialogRef so you only call setTariffId(this.selectedTariff.id) when
selectedTariff exists.
In @src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.ts:
- Around line 121-127: When hasPaymentLink is true, a failure in
cancelExistingPayment aborts processing; wrap
cancelExistingPayment(this.existingOrderId) with a catchError to recover and
return a harmless observable so processExistingOrder(...) still runs (import
catchError), e.g. catch the error, log or surface a user-friendly message via
the component logger/UI, and return of(null) (or EMPTY) so switchMap proceeds to
call orderService.processExistingOrder(this.getOrder(shouldBePaid),
this.existingOrderId); keep the hasPaymentLink, cancelExistingPayment,
processExistingOrder, getOrder, and existingOrderId references to locate the
change.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.ts (1)
72-72: Add explicit type conversion for query parameter.Query parameters from
ActivatedRouteare strings, butexistingOrderIdis typed asnumber. Convert the parameter explicitly usingNumber()or the unary+operator to ensure type safety and match the expected type fororderService.cancelExistingPayment()andorderService.processExistingOrder()which both expectnumber.Related pattern:
ubs-user-orders-list.component.ts(line 249) shows proper conversion withNumber(this.orderId).
🧹 Nitpick comments (2)
src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.ts (1)
125-126: Consider refactoring to reduce code duplication.The call to
processExistingOrderappears in both branches of the inneriif. This duplication could be reduced by restructuring the observable chain.🔎 Alternative approach without duplication
iif( () => this.existingOrderId >= 0, - iif( - () => this.hasPaymentLink, - this.orderService - .cancelExistingPayment(this.existingOrderId) - .pipe(switchMap(() => this.orderService.processExistingOrder(this.getOrder(shouldBePaid), this.existingOrderId))), - this.orderService.processExistingOrder(this.getOrder(shouldBePaid), this.existingOrderId) - ), + (this.hasPaymentLink + ? this.orderService.cancelExistingPayment(this.existingOrderId) + : of(null) + ).pipe(switchMap(() => this.orderService.processExistingOrder(this.getOrder(shouldBePaid), this.existingOrderId))), this.orderService.processNewOrder(this.getOrder(shouldBePaid)) )Note: You'll need to import
offromrxjs.src/app/store/effects/order.effects.ts (1)
77-88: Consider checking current route before navigation.The effect navigates to
/ubs/user/orderson 403 errors. If the user is already on that route, this could cause unnecessary re-rendering or route reinitialization.🔎 Proposed improvement to avoid redundant navigation
getExistingOrderDetailsFail = createEffect( () => this.actions.pipe( ofType(GetExistingOrderDetailsFail), filter(({ error }) => error.status === 403), tap(() => { this.snackBar.openSnackBar('errorOrderUnauthorized'); - this.router.navigate(['/ubs/user/orders']); + if (!this.router.url.includes('/ubs/user/orders')) { + this.router.navigate(['/ubs/user/orders']); + } }) ), { dispatch: false } );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/app/shared/services/mat-snack-bar/mat-snack-bar.service.tssrc/app/store/actions/order.actions.tssrc/app/store/effects/order.effects.tssrc/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.htmlsrc/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.spec.tssrc/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.tssrc/app/ubs/ubs/components/ubs-order-details/ubs-order-location-popup/ubs-order-location-popup.component.tssrc/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.tssrc/app/ubs/ubs/validators/address-validators.tssrc/assets/i18n/uk.json
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-11T20:44:21.136Z
Learnt from: witolDark
Repo: ita-social-projects/GreenCityClient PR: 3921
File: src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts:214-214
Timestamp: 2025-12-11T20:44:21.136Z
Learning: In Angular projects using HttpClient, HTTP call observables complete after emitting a single value. Therefore, take(1) or takeUntil is not strictly required to prevent memory leaks for typical single-emission requests. However, using take(1) (or takeUntil in combination with a component-destruction signal) can improve readability and guard against accidental multiple emissions if the observable is reused or transformed. For files like src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts and other services using HttpClient, rely on HttpClient completion for single requests, but consider adding take(1) when subscribing in long-lived contexts or when the observable could emit more than once due to retries or stream augmentation.
Applied to files:
src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.spec.tssrc/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.tssrc/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.tssrc/app/store/effects/order.effects.tssrc/app/store/actions/order.actions.tssrc/app/ubs/ubs/validators/address-validators.tssrc/app/ubs/ubs/components/ubs-order-details/ubs-order-location-popup/ubs-order-location-popup.component.tssrc/app/shared/services/mat-snack-bar/mat-snack-bar.service.ts
📚 Learning: 2025-08-01T22:32:58.734Z
Learnt from: AndrewKozovyi
Repo: ita-social-projects/GreenCityClient PR: 3770
File: src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts:103-110
Timestamp: 2025-08-01T22:32:58.734Z
Learning: In UbsAdminCustomersComponent (src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts), using take(1) instead of takeUntilDestroyed(this.destroyRef) for the adminTableOfCustomersSelector$ subscription prevents excessive additional functionality from being triggered and works correctly for the component's initialization sequence.
Applied to files:
src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.tssrc/app/ubs/ubs/components/ubs-order-details/ubs-order-location-popup/ubs-order-location-popup.component.ts
🧬 Code graph analysis (2)
src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.ts (2)
src/app/store/actions/order.actions.ts (4)
GetOrderDetails(94-94)GetExistingOrderDetails(109-109)GetCourierLocations(97-97)SetTariff(93-93)src/app/ubs/ubs/models/ubs.interface.ts (1)
ActiveTariffInfo(276-283)
src/app/store/effects/order.effects.ts (1)
src/app/store/actions/order.actions.ts (1)
GetExistingOrderDetailsFail(114-114)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (18.x)
🔇 Additional comments (12)
src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.ts (3)
4-4: LGTM!The
switchMapimport is necessary for the new conditional payment cancellation logic and is correctly imported.
41-41: LGTM!The
hasPaymentLinkfield appropriately tracks whether a payment link exists for conditional cancellation logic.
85-85: Previous issue resolved.The optional chaining has been correctly added, preventing potential runtime errors when
existingOrderInfois undefined.src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.html (1)
7-7: LGTM! Property reference updated correctly.The template correctly references
currentTariffNameinstead ofcurrentTariff, aligning with the component refactor where tariff ID and name are now tracked separately.src/app/store/actions/order.actions.ts (1)
5-5: LGTM! Proper error action implementation.The new
GetExistingOrderDetailsFailaction correctly follows NgRx patterns:
- Properly imports
HttpErrorResponsefor type safety- Adds the failure action to the enum
- Exports a typed action creator with the error payload
This enables downstream effects to handle 403 authorization errors when fetching existing order details.
Also applies to: 41-41, 114-114
src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.spec.ts (1)
381-381: LGTM! Test updated for property rename.The test assertion correctly references
component.currentTariffNameinstead ofcomponent.currentTariff, maintaining test coverage for the refactored tariff name storage.src/assets/i18n/uk.json (1)
517-517: LGTM! Translation entries added for error handling.The new Ukrainian translations support the 403 authorization error flow:
- Generic error message: "Сталася помилка"
- Order-specific authorization message: "Немає доступу до замовлення"
JSON syntax is correct with proper comma placement.
Also applies to: 533-534
src/app/shared/services/mat-snack-bar/mat-snack-bar.service.ts (1)
41-41: LGTM! Snackbar configuration updates align with error handling flow.The new
errorOrderUnauthorizedentry and the translation key addition toerrorMessagefollow existing patterns and support the 403 error handling introduced in the effects layer.Also applies to: 54-54
src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.ts (2)
57-58: LGTM! Cleaner separation of tariff id and name.The refactoring separates the tariff id (for logic) from the tariff name (for display), improving code clarity. The
currentTariffproperty now stores the numeric id, andcurrentTariffNamestores the localized name string.Also applies to: 161-162
164-166: Verify dispatches use correct tariffId parameter.The dispatches now use
tariffId: this.currentTariffwherecurrentTariffis a number (tariff id). Ensure the action signatures and effects expecttariffIdas a number, not a string.Based on the relevant code snippets,
GetOrderDetailsandGetCourierLocationsactions expect{ tariffId: number }, which aligns with the changes. The refactoring correctly updates these dispatches.src/app/ubs/ubs/validators/address-validators.ts (2)
3-3: KyivNamesEnum is properly defined with correct properties.The enum exists in
src/app/ubs/ubs/models/ubs.interface.tswith the expectedKyivEn('Kyiv') andKyivUa('Київ') properties. The import at line 3 is correct, and the validation logic at lines 50-52 properly uses these enum values to exclude Kyiv city from Kyiv region validation.
50-52: The AND condition is correct—no changes needed.Verification confirms the logic is sound. The strict AND condition (
cityEn === KyivEn && cityUk === KyivUa) appropriately requires both English and Ukrainian names to match before excluding an address, preventing false positives where only one name might coincide. The placement of this check after the locations list validation creates a good defensive safeguard.One observation: no unit tests exist for
KyivRegionValidatoror the address validation logic. Consider adding test coverage for edge cases including Kyiv city exclusion and distance-based validation.
src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.ts
Outdated
Show resolved
Hide resolved
.../components/ubs-order-details/ubs-order-location-popup/ubs-order-location-popup.component.ts
Show resolved
Hide resolved
src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.ts (2)
132-163: Missing error handling in the observable chain.The
getExistingOrderInfocall lacks error handling. If the request fails, the user will see no feedback, and the dialog won't open. Consider adding error handling to notify the user.🔎 Proposed fix
editOrPayPopup(order: IUserOrderInfo) { this.orderService .getExistingOrderInfo(order.id) .pipe( switchMap((info) => this.dialog .open(DialogPopUpComponent, { data: this.editOrPayDialogData, autoFocus: true }) .afterClosed() .pipe(map((res) => ({ res, info }))) ) ) - .subscribe(({ res, info }) => { + .subscribe({ + next: ({ res, info }) => { if (res) { if (info.paymentLink) { window.location.href = info.paymentLink; } else { this.openOrderPaymentPopUp(info); } return; } if (res === false) { if (this.isOrderHalfPaid(info)) { this.openOrderPaymentPopUp(info); } else { this.getDataForLocalStorage(info); } } - }); + }, + error: () => { + this.snackBarService.openSnackBar('error'); + } + }); }
188-234: No explicit error handling forforkJoinrequests.If either
getExistingOrderDetailsorgetPersonalDatafails, theforkJoinwill error and the subscription's success callback won't execute, leaving the user without feedback. Consider adding error handling.🔎 Proposed fix
- forkJoin([orderDataRequest, personalDataRequest]).subscribe(() => { + forkJoin([orderDataRequest, personalDataRequest]).subscribe({ + next: () => { this.bags = orderDataResponse.bags || []; // ... rest of the logic this.setDataToLocalStorage(); - }); + }, + error: () => { + this.snackBarService.openSnackBar('error'); + } + });src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.spec.ts (1)
477-490: Consider adding a test for the half-paid branch when dialog returnsfalse.This test covers the
getDataForLocalStoragebranch (whenisOrderHalfPaidis false), but there's no test for the case whenres === falseandisOrderHalfPaid(info)istrue, which should callopenOrderPaymentPopUp. Consider adding coverage for that scenario.🔎 Example test case
it('should call openOrderPaymentPopUp if dialog returned false and order is half-paid', fakeAsync(() => { dialogRefSpy.afterClosed.and.returnValue(of(false)); const halfPaidOrder = { ...fakeInputOrderData[1], paymentStatusEn: 'Half paid' }; orderServiceMock.getExistingOrderInfo.and.returnValue(of(halfPaidOrder)); const orderPaymentPopupSpy = spyOn(component as any, 'openOrderPaymentPopUp'); const getDataForLocalStorageSpy = spyOn(component, 'getDataForLocalStorage'); component.editOrPayPopup(fakeInputOrderData[1] as any); tick(); expect(orderPaymentPopupSpy).toHaveBeenCalledWith(halfPaidOrder as any); expect(getDataForLocalStorageSpy).not.toHaveBeenCalled(); }));
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.spec.tssrc/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.tssrc/app/ubs/ubs/components/ubs-order-details/ubs-order-location-popup/ubs-order-location-popup.component.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/ubs/ubs/components/ubs-order-details/ubs-order-location-popup/ubs-order-location-popup.component.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-11T20:44:21.136Z
Learnt from: witolDark
Repo: ita-social-projects/GreenCityClient PR: 3921
File: src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts:214-214
Timestamp: 2025-12-11T20:44:21.136Z
Learning: In Angular projects using HttpClient, HTTP call observables complete after emitting a single value. Therefore, take(1) or takeUntil is not strictly required to prevent memory leaks for typical single-emission requests. However, using take(1) (or takeUntil in combination with a component-destruction signal) can improve readability and guard against accidental multiple emissions if the observable is reused or transformed. For files like src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts and other services using HttpClient, rely on HttpClient completion for single requests, but consider adding take(1) when subscribing in long-lived contexts or when the observable could emit more than once due to retries or stream augmentation.
Applied to files:
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.spec.tssrc/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.ts
📚 Learning: 2025-08-01T22:32:58.734Z
Learnt from: AndrewKozovyi
Repo: ita-social-projects/GreenCityClient PR: 3770
File: src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts:103-110
Timestamp: 2025-08-01T22:32:58.734Z
Learning: In UbsAdminCustomersComponent (src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts), using take(1) instead of takeUntilDestroyed(this.destroyRef) for the adminTableOfCustomersSelector$ subscription prevents excessive additional functionality from being triggered and works correctly for the component's initialization sequence.
Applied to files:
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.ts
🧬 Code graph analysis (2)
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.spec.ts (1)
src/app/ubs/mocks/order-data-mock.ts (1)
fakeInputOrderData(108-161)
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.ts (1)
src/app/ubs/ubs/models/ubs.interface.ts (2)
OrderDetails(36-49)PersonalData(65-91)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (18.x)
🔇 Additional comments (6)
src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.ts (2)
232-232: Good use of optional chaining.The optional chaining on
order.id?.toString()adds defensive coding against potential undefined values.
148-151: No type compatibility issues exist.The
infoobject fromgetExistingOrderInfois correctly typed asIUserOrderInfo. The service method explicitly returnsObservable<IUserOrderInfo>, and bothopenOrderPaymentPopUp(order: IUserOrderInfo)andisOrderHalfPaid(order: IUserOrderInfo)accept this type. The types match exactly and no runtime errors will occur.src/app/ubs/ubs-user/components/ubs-user-orders-list/ubs-user-orders-list.component.spec.ts (4)
92-102: Mock setup looks correct.The
getExistingOrderInfomethod is properly added to the mock and configured with a default return value. This aligns with the component's new data flow.
391-391: Good typo fix.Corrected "orsers" to "orders" in the test description.
462-475: Test correctly verifies thetruedialog result branch.The mock for
getExistingOrderInforeturns the expected order data, and the assertions verify thatopenOrderPaymentPopUpis called with the info from the service response.
233-245: Updated test data reference aligns with the new flow.The test now correctly uses
fakeInputOrderData[0]and expectsorderId: 3, which matches the data structure. This ensures the test reflects the actual component behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.spec.ts (1)
31-32: Add an assertion for the new cancellation branch (and type the spy to catch signature drift).Right now you mock
cancelExistingPayment, but no test verifies it’s called when the “has payment link → cancel” path is taken, so the suite may not actually protect this new behavior.Proposed tweak (type-safe spy + minimal assertion)
- const orderServiceSpy = jasmine.createSpyObj('OrderService', ['processExistingOrder', 'processNewOrder', 'cancelExistingPayment']); + const orderServiceSpy = jasmine.createSpyObj<OrderService>('OrderService', [ + 'processExistingOrder', + 'processNewOrder', + 'cancelExistingPayment' + ]); orderServiceSpy.cancelExistingPayment.and.returnValue(of(true));it('should process existing order successfully', () => { orderService.processExistingOrder.and.returnValue(of({ orderId: 123, link: 'https://' })); component.existingOrderId = 1; component.personalData = personalMockData; component.orderDetails = orderDetailsMock; component.processOrder(true); expect(component.isLoadingAnim).toBe(true); + // If the new flow cancels when a payment link exists, assert it here (adjust params as per real signature). + expect(orderService.cancelExistingPayment).toHaveBeenCalled(); expect(orderService.processExistingOrder).toHaveBeenCalled(); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/ubs/ubs/components/ubs-order-details/ubs-order-location-popup/ubs-order-location-popup.component.spec.tssrc/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.spec.ts
💤 Files with no reviewable changes (1)
- src/app/ubs/ubs/components/ubs-order-details/ubs-order-location-popup/ubs-order-location-popup.component.spec.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-01T22:32:58.734Z
Learnt from: AndrewKozovyi
Repo: ita-social-projects/GreenCityClient PR: 3770
File: src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts:103-110
Timestamp: 2025-08-01T22:32:58.734Z
Learning: In UbsAdminCustomersComponent (src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts), using take(1) instead of takeUntilDestroyed(this.destroyRef) for the adminTableOfCustomersSelector$ subscription prevents excessive additional functionality from being triggered and works correctly for the component's initialization sequence.
Applied to files:
src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.spec.ts
📚 Learning: 2025-12-11T20:44:21.136Z
Learnt from: witolDark
Repo: ita-social-projects/GreenCityClient PR: 3921
File: src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts:214-214
Timestamp: 2025-12-11T20:44:21.136Z
Learning: In Angular projects using HttpClient, HTTP call observables complete after emitting a single value. Therefore, take(1) or takeUntil is not strictly required to prevent memory leaks for typical single-emission requests. However, using take(1) (or takeUntil in combination with a component-destruction signal) can improve readability and guard against accidental multiple emissions if the observable is reused or transformed. For files like src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts and other services using HttpClient, rely on HttpClient completion for single requests, but consider adding take(1) when subscribing in long-lived contexts or when the observable could emit more than once due to retries or stream augmentation.
Applied to files:
src/app/ubs/ubs/components/ubs-submit-order/ubs-submit-order.component.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (18.x)
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts:
- Around line 213-311: The test "should handle non-Date object by converting to
Date" is flaky because parsing '2024-06-15' can shift across timezones; update
the test to avoid reliance on string-to-Date parsing by providing a
deterministic Date instance or by passing explicit numeric parts: replace
dateString = '2024-06-15' and dateEvent.value with new Date(2024, 5, 15) (or
build via parts and new Date(year, month-1, day)), keep
component.id/nameOfColumn and the expectation against
component.editDateCell.emit unchanged so changeData() receives an unambiguous
Date and the emitted newValue remains '2024-06-15'.
- Around line 359-396: The test for the blocked-orders branch has an incorrect
assertion: when adminTableService.blockOrders returns a blocked result
(iAlertInfo), the component.edit branch should call
component.showBlockedInfo.emit(res) and NOT open the datepicker; remove the
assertion expecting mockDatepicker.open toHaveBeenCalled in that test. Locate
the second spec in the "Integration scenarios" block that spies on
adminTableService.blockOrders and component.showBlockedInfo, then delete the
expect(mockDatepicker.open).toHaveBeenCalled() line so the test only asserts
blockOrders was called, showBlockedInfo.emit was called with iAlertInfo, and the
cursor wait dispatch happened.
In @src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.ts:
- Around line 349-356: existingOrderId is declared as a number but the dialog
handler assigns null to it, causing TypeScript strict-null checks to fail;
update the existingOrderId property declaration on the component
(existingOrderId) to type number | null and adjust any initializations/usages
(e.g., where existingOrderId is read or assigned from orderId: number | null) to
handle possible null values so assignments like this.existingOrderId = null are
type-safe.
🧹 Nitpick comments (2)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts (2)
41-66: Stabilize the “current date” test by using a fixed mocked date (notnew Date()).
Right now it’s likely fine because both sides use the mocked clock, but using a constant makes the test intention clearer and eliminates any clock/mock timing edge.Proposed tweak
beforeEach(() => { jasmine.clock().install(); - jasmine.clock().mockDate(new Date()); + jasmine.clock().mockDate(new Date(2024, 0, 1, 12, 0, 0)); ... }); ... it('should set current date on initialization', () => { - const mockDate = new Date(); - expect(component.current).toEqual(mockDate); + expect(component.current).toEqual(new Date(2024, 0, 1, 12, 0, 0)); });
74-211: AssertSetCursorWaitedispatch call order (optional), not just presence.
If the component ever dispatches additional actions,toHaveBeenCalledWith(...)can still pass while the UX regresses (e.g.,falsedispatched beforetrue).Example pattern
component.edit(mockDatepicker as MatDatepicker<Date>); - expect(store.dispatch).toHaveBeenCalledWith(SetCursorWaite({ isWaiting: true })); + expect(store.dispatch.calls.allArgs()).toEqual([ + [SetCursorWaite({ isWaiting: true })], + [SetCursorWaite({ isWaiting: false })] + ]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.tssrc/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-01T22:32:58.734Z
Learnt from: AndrewKozovyi
Repo: ita-social-projects/GreenCityClient PR: 3770
File: src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts:103-110
Timestamp: 2025-08-01T22:32:58.734Z
Learning: In UbsAdminCustomersComponent (src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts), using take(1) instead of takeUntilDestroyed(this.destroyRef) for the adminTableOfCustomersSelector$ subscription prevents excessive additional functionality from being triggered and works correctly for the component's initialization sequence.
Applied to files:
src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
📚 Learning: 2025-12-11T20:44:21.136Z
Learnt from: witolDark
Repo: ita-social-projects/GreenCityClient PR: 3921
File: src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts:214-214
Timestamp: 2025-12-11T20:44:21.136Z
Learning: In Angular projects using HttpClient, HTTP call observables complete after emitting a single value. Therefore, take(1) or takeUntil is not strictly required to prevent memory leaks for typical single-emission requests. However, using take(1) (or takeUntil in combination with a component-destruction signal) can improve readability and guard against accidental multiple emissions if the observable is reused or transformed. For files like src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts and other services using HttpClient, rely on HttpClient completion for single requests, but consider adding take(1) when subscribing in long-lived contexts or when the observable could emit more than once due to retries or stream augmentation.
Applied to files:
src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.tssrc/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
🧬 Code graph analysis (2)
src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.ts (2)
src/app/store/actions/order.actions.ts (4)
GetOrderDetails(94-94)GetExistingOrderDetails(109-109)GetCourierLocations(97-97)SetTariff(93-93)src/app/ubs/ubs/models/ubs.interface.ts (1)
ActiveTariffInfo(276-283)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts (1)
src/app/store/actions/ubs-admin.actions.ts (1)
SetCursorWaite(10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (18.x)
🔇 Additional comments (2)
src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.ts (2)
57-58: LGTM! Good separation of ID and display name.The refactor to separate
currentTariff(ID) as a number fromcurrentTariffName(display) as a string improves type safety and clarity. This aligns well with theActiveTariffInfointerface and the action signatures that expect numerictariffIdvalues.
160-165: Verified:getTariffNamemethod exists and is correctly implemented.The
OrderService.getTariffName()method is properly defined at line 240 insrc/app/ubs/ubs/services/order.service.ts, takes anActiveTariffInfoparameter, and returns a localized tariff name string vialangService.getLangValue(). The tariff handling logic at lines 160-165 correctly extracts the ID and name, dispatches type-safe numerictariffIdvalues, and appropriately handles both existing and new orders.
...p/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
Show resolved
Hide resolved
...p/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
Show resolved
Hide resolved
src/app/ubs/ubs/components/ubs-order-details/ubs-order-details.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts (2)
74-80: Consider testing locale effects via public interface.While the test correctly verifies that
adapter.setLocaleis called, accessing private properties with bracket notation (component['adapter']) couples tests to implementation details. If feasible, consider testing the observable effect of locale changes through the public API instead.
282-302: Verify that console.error is sufficient for invalid date handling.The tests verify that
console.erroris called for null/undefined date values. While this provides debugging output, consider whether the component should:
- Throw an error to fail fast
- Emit a specific error event
- Display a user-facing error message
Console errors are easily missed in production and may not provide adequate error recovery.
Do you want me to suggest a more robust error handling approach?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-01T22:32:58.734Z
Learnt from: AndrewKozovyi
Repo: ita-social-projects/GreenCityClient PR: 3770
File: src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts:103-110
Timestamp: 2025-08-01T22:32:58.734Z
Learning: In UbsAdminCustomersComponent (src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts), using take(1) instead of takeUntilDestroyed(this.destroyRef) for the adminTableOfCustomersSelector$ subscription prevents excessive additional functionality from being triggered and works correctly for the component's initialization sequence.
Applied to files:
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
📚 Learning: 2025-12-11T20:44:21.136Z
Learnt from: witolDark
Repo: ita-social-projects/GreenCityClient PR: 3921
File: src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts:214-214
Timestamp: 2025-12-11T20:44:21.136Z
Learning: In Angular projects using HttpClient, HTTP call observables complete after emitting a single value. Therefore, take(1) or takeUntil is not strictly required to prevent memory leaks for typical single-emission requests. However, using take(1) (or takeUntil in combination with a component-destruction signal) can improve readability and guard against accidental multiple emissions if the observable is reused or transformed. For files like src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-select/table-cell-select.component.ts and other services using HttpClient, rely on HttpClient completion for single requests, but consider adding take(1) when subscribing in long-lived contexts or when the observable could emit more than once due to retries or stream augmentation.
Applied to files:
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (18.x)
🔇 Additional comments (6)
src/app/ubs/ubs-admin/components/ubs-admin-table/table-cell-date/table-cell-date.component.spec.ts (6)
1-56: LGTM! Well-structured test setup.The test configuration is comprehensive with appropriate mocks for the MatDatepicker, MockStore, and ChangeDetectorRef. The jasmine clock setup for date testing is a good practice.
58-72: LGTM! Proper initialization coverage.The initialization tests appropriately verify component creation and default state setup.
82-219: Excellent coverage of edit flow, including keyboard accessibility.The edit() method tests comprehensively cover:
- Early returns for uneditable states
- Keyboard navigation (Enter/Space keys) for accessibility
- Loading state management via SetCursorWaite
- All branching logic for order selection (isAllChecked, ordersToChange, id)
- Datepicker interaction and blocked order handling
The keyboard event tests (lines 98-132) are particularly valuable for ensuring accessibility compliance.
221-280: LGTM! Comprehensive date formatting coverage.The tests thoroughly verify:
- ISO 8601 date format (yyyy-MM-dd) with leading zeros
- Edge cases: start of year (01-01) and end of year (12-31)
- Proper IEditCell object emission with id, nameOfColumn, and newValue
The date formatting validation is particularly robust.
321-365: LGTM! Modern Angular testing approach for Input properties.The tests use
fixture.componentRef.setInput(), which is the recommended Angular 14+ approach for testing @input() properties. All input properties are covered:
- date, nameOfColumn, id
- ordersToChange, isAllChecked
- uneditableStatus
367-404: Excellent end-to-end integration coverage.The integration tests verify complete workflows:
- Full edit flow when orders are not blocked (datepicker opens, cursor states managed)
- Full edit flow when orders are blocked (showBlockedInfo emitted, proper cleanup)
These tests provide valuable regression protection by testing multiple components working together (store, service, datepicker, change detection).
|
* changed view encapsulation from none to emulated in main page comp to fix styles leakage * Bugfix/table cell date and address fix (#3930) * Bugfix/table col width fix, changed pwa app name (#3929) * changed pwa app name * admin table row styles * Bugfix #8972 profile deactivation (#3925) * Fix #8972 changed profile deactivation base url, changed a request body format * Fix #8972 changed profile deactivation base url, changed a request body format * Bugfix/added technical support block to main page and fixed styles (#3932) * added technical support block for main page * fixed translation for new tech support quills, fixed styles for main page * tests fix * Fix/frontend bugs (#3931) * Add snack-bar instead 'offline' field in header * Add filter tags for Customers * Fix sorting in customers orders table * Remove coma * Add tests for snack bar and filter tags * Fix coderabbit issues * Fix coderabbit issues * Remove commas --------- Co-authored-by: vryazh <vryazhskaya@gmail.com> Co-authored-by: Saienko Volodymyr <48810972+SaenkoVova@users.noreply.github.com> Co-authored-by: AnastasiaRakuta <168209962+AnastasiaRakuta@users.noreply.github.com>
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Style
✏️ Tip: You can customize this high-level summary in your review settings.