Fix/frontend bugs #3931
Merged
Fix/frontend bugs #3931
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughThese changes replace the offline status UI banner with snackbar notifications, refactor customer filtering to display active filters with removal capability, and swap sorting direction on header icons for improved UX clarity. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (1)
105-120: Fix CI: remove trailing comma in constructor params (ESLint comma-dangle).This is currently breaking the pipeline.
Proposed fix
constructor( @@ private readonly adminCustomerService: AdminCustomersService, - private readonly translate: TranslateService, + private readonly translate: TranslateService ) {}
🤖 Fix all issues with AI agents
In
@src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.html:
- Around line 288-293: The remove control is an interactive element implemented
as a non-focusable <div>; update the template for the tag filter (the block
using *ngFor over activeFilters) so the clickable element becomes an actual
accessible control: replace the remove-item-btn div with a <button> (or at
minimum add role="button", tabindex="0", keyboard handlers for Enter/Space and
an aria-label) that calls removeActiveFilter(filter) on click; ensure the button
has an accessible name (e.g., aria-label="{{ 'REMOVE_FILTER' | translate }} {{
filter.labelKey | translate }}") and keep existing styling classes so appearance
remains unchanged.
In
@src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.scss:
- Around line 427-436: The .remove-item-btn rule uses an absolute '/assets/...'
path and a tiny 6×6px hit target; update the background URL to a relative path
that respects base-href (e.g., url("assets/img/ubs-admin-employees/cross.svg")),
make the element at least 24×24px for accessibility (width/height: 24px), keep
the visible icon smaller via background-size (e.g., 12px) and center it with
background-position: center; also ensure display: inline-block,
background-repeat: no-repeat, cursor: pointer and remove any default border so
the larger clickable area doesn’t change layout or visuals.
🧹 Nitpick comments (4)
src/app/app.component.ts (2)
29-32: Removeonline/offlinelisteners on destroy (and avoid.bind(this)so it’s removable).Right now you add event listeners but never remove them, and the
.bind(this)creates a new function reference that can’t be removed later. Consider stable handlers + cleanup; you can also wireSwUpdate.versionUpdateswithtakeUntilDestroyed(...)for consistency.Proposed fix
export class AppComponent implements OnInit, OnDestroy { @@ private readonly destroy$: Subject<void> = new Subject<void>(); + private readonly onOnline = () => this.onNetworkStatusChange(); + private readonly onOffline = () => this.onNetworkStatusChange(); @@ ngOnInit(): void { @@ this.onNetworkStatusChange(); - window.addEventListener('online', this.onNetworkStatusChange.bind(this)); - window.addEventListener('offline', this.onNetworkStatusChange.bind(this)); + window.addEventListener('online', this.onOnline); + window.addEventListener('offline', this.onOffline); @@ ngOnDestroy(): void { + window.removeEventListener('online', this.onOnline); + window.removeEventListener('offline', this.onOffline); this.destroy$.next(); this.destroy$.complete(); } }Also applies to: 48-52, 54-57
48-52: Avoid snackbar spam when flapping/offline fires multiple times.If
offlinefires repeatedly (or user refreshes while offline),openSnackBar('noInternet')may stack notifications. If yourMatSnackBarServicedoesn’t already dedupe, consider dismissing an existing one / “open once” behavior / adding a duration.src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (2)
83-84: Tighten types foractiveFiltersandremoveActiveFilter(and avoidmapas a variable name).This will reduce template/TS drift and make
removeActiveFilter(filter)safer.Possible refactor
export class UbsAdminCustomersComponent implements OnInit, AfterViewChecked, OnDestroy { @@ - activeFilters: Array<{key: string; labelKey: string;valueText: string; fromControl: string; toControl: string;}> = []; + activeFilters: Array<{ + key: string; + labelKey: string; + valueText: string; + fromControl: string; + toControl: string; + }> = []; @@ private buildActiveFilters(): void { @@ - const map = [ + const filterDefs = [ @@ - map.forEach((f) => { + filterDefs.forEach((f) => { @@ } -removeActiveFilter(filter): void { +removeActiveFilter(filter: { key: string; fromControl: string; toControl: string }): void { this.filterForm.get(filter.fromControl).setValue(''); this.filterForm.get(filter.toControl).setValue(''); this.activeFilters = this.activeFilters.filter(f => f.key !== filter.key); this.submitFilterForm(); }Also applies to: 266-343, 345-350
263-264: Nice UX:buildActiveFilters()aftergetTable()is fine; consider avoiding forced lowercasing.
toLowerCase()on translated “from/to” can look odd in some languages (and is unnecessary if the translation is already in desired case).Also applies to: 266-343
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/app.component.htmlsrc/app/app.component.scsssrc/app/app.component.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customer-orders/ubs-admin-customer-orders.component.htmlsrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.htmlsrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.scsssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts
💤 Files with no reviewable changes (2)
- src/app/app.component.scss
- src/app/app.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-customers/ubs-admin-customers.component.htmlsrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.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/app.component.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts
🪛 GitHub Actions: CI/CD GreenCityClient
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts
[error] 118-118: ESLint: Unexpected trailing comma. (comma-dangle)
🪛 GitHub Check: build (18.x)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts
[failure] 118-118:
Unexpected trailing comma
🔇 Additional comments (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customer-orders/ubs-admin-customer-orders.component.html (1)
21-26: Confirm icon semantics match actual sort direction (ASC/DESC).You swapped the
ASC/DESCpassed intoonSortTable(...); please double-check thatarrow_downwardnow really corresponds to ascending (andarrow_upwardto descending) per your UX convention, and thatarrowDirection(which looks column-based) still lets users toggle direction predictably.
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.html
Show resolved
Hide resolved
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.scss
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts:
- Around line 266-343: buildActiveFilters currently assumes date form values are
JS Date and lowercases translated words; rename the local const map to
filterDefs for clarity, detect Moment values from filterForm (e.g., if
from?.toDate is a function) and call from = datePipe.transform(from.toDate(),
'dd/MM/yyyy') (and similarly for to) instead of new Date(from), and remove the
locale-sensitive .toLowerCase() calls on
translate.instant('ubs-customer-filters.from')/('...to') so you use the
translations as provided; update references from map to filterDefs and keep the
rest of the activeFilters construction unchanged.
- Around line 117-119: The active filter labels are not rebuilt when the app
language changes; subscribe to the language change observable
(languageBehaviourSubject) and call buildActiveFilters() inside that
subscription so the valueText generated with translate.instant() is regenerated;
locate the existing languageBehaviourSubject subscription (around where
languageBehaviourSubject is used) and add a call to this.buildActiveFilters()
(similar to ubs-admin-table's formatTableData on language change) to ensure
filter display tags update when TranslateService language changes.
🧹 Nitpick comments (2)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (2)
83-85: ExtractactiveFilterstype into a named interface (and fix formatting).Line 83’s inline object type is hard to read/maintain (also missing a space after
;beforevalueText). A smallinterface ActiveFilterTag { ... }will makeremoveActiveFilter()and template bindings safer.
345-350: Remove redundantactiveFiltersmutation;submitFilterForm()already rebuilds tags.Line 348 is unnecessary (and could cause a brief UI mismatch) since Line 349 calls
submitFilterForm()which callsbuildActiveFilters()and recalculates the array.Proposed change
removeActiveFilter(filter): void { this.filterForm.get(filter.fromControl).setValue(''); this.filterForm.get(filter.toControl).setValue(''); - this.activeFilters = this.activeFilters.filter(f => f.key !== filter.key); this.submitFilterForm(); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.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-admin/components/ubs-admin-customers/ubs-admin-customers.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-customers/ubs-admin-customers.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-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (2)
226-265: Good: always rebuild tag UI after applying filters (even when query didn’t change).Calling
buildActiveFilters()unconditionally (Line 263) keeps UI tags consistent with the current form state—nice.
37-39: Avoid instantiatingDatePipeon every call; useformatDate()instead.Line 309 creates
new DatePipe(locale)insidebuildActiveFilters(), which is called on each filter submission. For programmatic date formatting (outside templates), useformatDate()from@angular/common:import { formatDate } from '@angular/common'; // In buildActiveFilters(): const formattedDate = formatDate(from, 'dd/MM/yyyy', locale);Alternatively, inject
DatePipeonce in the constructor and reuse it, passing the locale totransform()each time.⛔ Skipped due to learnings
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.
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts
Outdated
Show resolved
Hide resolved
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.spec.ts (1)
338-357: Verify thatsubmitFilterFormis called after removing the filter.The test sets up a spy on
submitFilterFormbut never asserts it was called. Based on the component's behavior (whereremoveActiveFilterre-submits the filter form), add an assertion to verify the spy.✅ Suggested enhancement
it('should remove active filter and reset form controls', () => { spyOn(component, 'submitFilterForm'); component.activeFilters = [ { key: 'bonuses', labelKey: 'ubs-customer-filters.bonuses', valueText: 'from 10 to 20', fromControl: 'bonusesFrom', toControl: 'bonusesTo' } ]; component.filterForm.patchValue({ bonusesFrom: 10, bonusesTo: 20 }); component.removeActiveFilter(component.activeFilters[0]); expect(component.activeFilters.length).toBe(0); expect(component.filterForm.value.bonusesFrom).toBe(''); expect(component.filterForm.value.bonusesTo).toBe(''); + expect(component.submitFilterForm).toHaveBeenCalled(); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/app.component.spec.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.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-customers/ubs-admin-customers.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-customers/ubs-admin-customers.component.spec.tssrc/app/app.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 (2)
src/app/app.component.spec.ts (2)
14-14: LGTM! Clean integration of MatSnackBarService mock.The MatSnackBarService mock is properly set up with a spy object and correctly provided in the TestBed configuration.
Also applies to: 23-23, 37-37, 55-56
72-82: LGTM! Tests correctly verify network status snackbar behavior.Both tests properly mock
navigator.onLineand verify that the snackbar is shown only when offline, aligning with the PR objective to replace the offline banner with a snackbar notification.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@src/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.ts:
- Line 9: The file ending with a closing brace (the interface in
customers-filters.interface.ts) is missing a trailing newline which violates the
eol-last ESLint rule; open the customers-filters.interface.ts file (the
interface declaration ending with “}”) and add a single newline character at the
end of the file so the file ends with a blank line.
In
@src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts:
- Line 37: Remove the unused TranslateService from the
UbsAdminCustomersComponent: delete the import line for TranslateService and
remove the corresponding constructor parameter (the injected TranslateService
instance) from the component's constructor signature and any related parameter
property; ensure no code references remain to TranslateService inside
UbsAdminCustomersComponent so the template can continue using the | translate
pipe without the service injection.
🧹 Nitpick comments (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (1)
267-329: Consider optimizing DatePipe instantiation.A new
DatePipeinstance is created on every call tobuildActiveFilters()(line 306). Consider creating it once as a class member or injecting it via the constructor to improve performance, especially if filters are updated frequently.♻️ Proposed optimization
Add DatePipe as a class dependency:
constructor( private readonly adapter: DateAdapter<any>, public readonly dialog: MatDialog, private readonly fb: FormBuilder, private readonly cdr: ChangeDetectorRef, private readonly renderer: Renderer2, private readonly router: Router, private readonly store: Store, private readonly destroyRef: DestroyRef, private readonly convertFromDateToStringService: ConvertFromDateToStringService, private readonly localStorageService: LocalStorageService, private readonly tableHeightService: TableHeightService, private readonly adminCustomerService: AdminCustomersService, - private readonly translate: TranslateService + private readonly translate: TranslateService, + private readonly datePipe: DatePipe ) {}Then update
buildActiveFilters()to use the injected instance:private buildActiveFilters(): void { this.activeFilters = []; const map = [ // ... map configuration ]; const locale = this.currentLang === 'uk' ? 'uk-UA' : 'en-GB'; - const datePipe = new DatePipe(locale); map.forEach((f) => { let from = this.filterForm.get(f.from).value; let to = this.filterForm.get(f.to).value; if (f.isDate) { - from = from ? datePipe.transform(new Date(from), 'dd/MM/yyyy') : from; - to = to ? datePipe.transform(new Date(to), 'dd/MM/yyyy') : to; + from = from ? this.datePipe.transform(new Date(from), 'dd/MM/yyyy', undefined, locale) : from; + to = to ? this.datePipe.transform(new Date(to), 'dd/MM/yyyy', undefined, locale) : to; } // ... rest of the method }); }Note: Pass the locale as the fourth parameter to
transform()to respect the current language.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.htmlsrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.scsssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.spec.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.scss
- src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.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-customers/ubs-admin-customers.component.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.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-customers/ubs-admin-customers.component.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.spec.ts
🧬 Code graph analysis (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.ts (1)
FilterCustomers(1-9)
🪛 GitHub Actions: CI/CD GreenCityClient
src/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.ts
[error] 9-9: ESLint: Newline required at end of file (eol-last).
🪛 GitHub Check: build (18.x)
src/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.ts
[failure] 9-9:
Newline required at end of file but not found
🔇 Additional comments (2)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.spec.ts (1)
338-358: LGTM! Well-structured test for active filter removal.The test comprehensively validates the
removeActiveFilterfunctionality by verifying that the active filter is removed from the array, form controls are reset to empty strings, and the filter form is resubmitted.src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (1)
331-336: LGTM! Clean implementation of filter removal.The
removeActiveFiltermethod correctly resets both form controls, removes the filter from the active list, and triggers form resubmission to update the table data.
src/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.ts
Outdated
Show resolved
Hide resolved
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts
Outdated
Show resolved
Hide resolved
softservedata
approved these changes
Jan 9, 2026
Contributor
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 (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (1)
105-118: Fix CI “Unexpected trailing comma” in constructor params.This matches the failing check at Line 117; please remove the trailing comma after the last parameter.
Proposed fix
constructor( private readonly adapter: DateAdapter<any>, public readonly dialog: MatDialog, private readonly fb: FormBuilder, private readonly cdr: ChangeDetectorRef, private readonly renderer: Renderer2, private readonly router: Router, private readonly store: Store, private readonly destroyRef: DestroyRef, private readonly convertFromDateToStringService: ConvertFromDateToStringService, private readonly localStorageService: LocalStorageService, private readonly tableHeightService: TableHeightService, - private readonly adminCustomerService: AdminCustomersService, + private readonly adminCustomerService: AdminCustomersService ) {}
🤖 Fix all issues with AI agents
In @src/app/app.component.spec.ts:
- Line 52: Remove the trailing comma after the SwUpdate provider entry in the
TestBed providers array (the line containing "{ provide: SwUpdate, useValue: {}
}") so it complies with the project's comma-dangle ESLint rule; update the
providers list to end that entry without a comma.
🧹 Nitpick comments (3)
src/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.ts (1)
1-9: Consider constrainingfromControl/toControlto real form keys (type-safety).Right now they’re plain
string; if feasible, tighten them to something likekeyof Filters(or the form model type) to prevent typos from compiling.src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (2)
225-327: Rename localmapand consider avoiding per-callDatePipecreation.
const map = [...]is easy to confuse with the imported RxJSmap; a name likefilterDefsreads clearer.- If this runs frequently, you can also hoist the definitions (and optionally the pipe) to avoid re-allocations.
Also please verify the app registers locale data for
uk-UA; otherwiseDatePipecan throw at runtime when formatting. (Angular i18n behavior can vary by setup/version.)
329-334: TyperemoveActiveFilterparam and add light guards for form controls.This is a public method called by the template; typing it as
FilterCustomersand guardingthis.filterForm.get(...)results makes it harder to break accidentally.Proposed fix
-removeActiveFilter(filter): void { - this.filterForm.get(filter.fromControl).setValue(''); - this.filterForm.get(filter.toControl).setValue(''); - this.activeFilters = this.activeFilters.filter(f => f.key !== filter.key); - this.submitFilterForm(); -} +public removeActiveFilter(filter: FilterCustomers): void { + this.filterForm.get(filter.fromControl)?.setValue(''); + this.filterForm.get(filter.toControl)?.setValue(''); + this.submitFilterForm(); // rebuilds activeFilters from the form state +}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/app.component.spec.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.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-admin/components/ubs-admin-customers/customers-filters.interface.tssrc/app/app.component.spec.tssrc/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.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-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts
🧬 Code graph analysis (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.ts (1)
FilterCustomers(1-9)
🪛 GitHub Actions: CI/CD GreenCityClient
src/app/app.component.spec.ts
[error] 52-52: ESLint: Unexpected trailing comma. Rule: comma-dangle
🪛 GitHub Check: build (18.x)
src/app/app.component.spec.ts
[failure] 52-52:
Unexpected trailing comma
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts
[failure] 117-117:
Unexpected trailing comma
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (2)
303-304: Consider injecting or cachingDatePipeinstead of instantiating on every call.Creating a new
DatePipeinstance each timebuildActiveFilters()is called is inefficient. SincecurrentLangcan change, you could either:
- Inject
DatePipevia DI (requires adding to providers)- Cache the pipe as a class property and recreate only when locale changes
♻️ Suggested approach using DI
Add to component providers and constructor:
// In @Component decorator providers: [ DatePipe, // ... existing providers ] // In constructor constructor( private readonly datePipe: DatePipe, // ... existing injections ) {}Then in
buildActiveFilters():- const locale = this.currentLang === 'uk' ? 'uk-UA' : 'en-GB'; - const datePipe = new DatePipe(locale); + // DatePipe uses the locale from LOCALE_ID or can be configured in providersNote: If locale needs to change dynamically, consider caching the pipe instance as a class property and recreating only when
currentLangchanges in the language subscription.
329-334: Add type annotation and remove redundant array manipulation.
- The
filterparameter should be typed asFilterCustomersfor type safety.- Line 332 is redundant since
submitFilterForm()callsbuildActiveFilters()which rebuilds the entireactiveFiltersarray from form values. After resetting the form controls, the filter won't appear in the rebuilt array.♻️ Proposed fix
-removeActiveFilter(filter): void { +removeActiveFilter(filter: FilterCustomers): void { this.filterForm.get(filter.fromControl).setValue(''); this.filterForm.get(filter.toControl).setValue(''); - this.activeFilters = this.activeFilters.filter(f => f.key !== filter.key); this.submitFilterForm(); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.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-admin/components/ubs-admin-customers/ubs-admin-customers.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-customers/ubs-admin-customers.component.ts
🧬 Code graph analysis (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (1)
src/app/ubs/ubs-admin/components/ubs-admin-customers/customers-filters.interface.ts (1)
FilterCustomers(1-9)
⏰ 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 (3)
src/app/ubs/ubs-admin/components/ubs-admin-customers/ubs-admin-customers.component.ts (3)
37-38: LGTM!Imports are appropriate for the new active filters functionality.
83-83: LGTM!Property is correctly typed and initialized.
262-263: LGTM!The call to
buildActiveFilters()is correctly placed to refresh active filter tags after form submission.
|
witolDark
approved these changes
Jan 13, 2026
softservedata
approved these changes
Jan 14, 2026
softservedata
pushed a commit
that referenced
this pull request
Jan 16, 2026
* 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
GreenCityClient PR
Issue Link 📋
#9022
#9303
#9305
Changed
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.