Added classes by maximgorbatyuk · Pull Request #113 · Techinterview-space/frontend

maximgorbatyuk · 2026-01-04T11:45:48Z

Summary by CodeRabbit

Release Notes

New Features
- Added exchange rates management interface to the admin panel with create, read, and delete operations
- Introduced pagination and improved loading states for the currencies admin page
- Added a new currencies chart visualization in the Salaries section displaying weekly exchange rate trends
- Added "Курсы валют" navigation link in the Salaries dropdown menu

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-04T11:45:57Z

Walkthrough

This PR introduces a currencies management feature with an admin page for CRUD operations and pagination, a salaries module chart page for visualizing weekly currency data, a new service layer for API integration, and navbar integration to link to the new chart view.

Changes

Cohort / File(s)	Summary
Admin Currencies Management `src/app/modules/admin/components/currencies-page/currencies-page.component.ts`, `currencies-page.component.html`, `currencies-page.component.scss`, `currencies-page.component.spec.ts`, `currency-item.ts`, `currency-record-edit-form.ts`	Replaces old admin tools flow with new currencies-centric CRUD operations. Adds pagination via `loadData()`, edit/create/delete dialogs, and currency details modal. Updates `CurrencyItem` to implement `CurrencyRecord` instead of `CurrencyData`. Introduces `CurrencyRecordEditForm` for managing form controls. HTML template adds loading states, pagination controls, confirm dialogs, and modals for create/edit/view operations. SCSS adds styling for JSON display block. Spec file updates service mocks.
Salaries Chart Component `src/app/modules/salaries/components/currencies-chart-page/currencies-chart-page.component.ts`, `currencies-chart-page.component.html`, `currencies-chart-page.component.scss`, `currencies-chart-page.component.spec.ts`, `currencies-chart-object.ts`	Adds new component for displaying weekly currency chart data with loading/error states. `CurrenciesChartPageComponent` manages data fetching, chart initialization, and lifecycle. `CurrenciesChartObject` wraps Chart.js configuration with multi-currency line chart. HTML template includes refresh button, loading indicator, error display, and canvas rendering. SCSS styles chart container. Spec adds basic tests for component creation and data population.
Service Layer `src/app/services/currencies-collection.service.ts`	New service providing currency CRUD operations (`getAll`, `create`, `delete`) with pagination support and chart data endpoint (`getChartData`). Exports interfaces: `CurrencyRecord`, `CreateCurrencyRecordRequest`, `CurrencyItemDto`, `WeeklyCurrencyChartData`, `CurrencyChartResponse`. Uses `ApiService` and `ConvertObjectToHttpParams` for HTTP integration.
Module & Routing `src/app/modules/salaries/salaries-routing.module.ts`, `src/app/modules/salaries/salaries.module.ts`	Adds new route "currencies-chart" and declares `CurrenciesChartPageComponent` in `SalariesModule`.
Navbar `src/app/components/navbar/navbar.component.ts`	Adds new menu item "Курсы валют" under "Зарплаты" (Salaries) dropdown linking to `/salaries/currencies-chart`.

Sequence Diagrams

sequenceDiagram
    participant User as User
    participant AdminUI as Admin Page
    participant Service as CurrenciesService
    participant API as Backend API
    participant Dialog as Dialog/Modal

    Note over User,API: Currency Record CRUD Flow
    
    User->>AdminUI: Opens currencies page
    AdminUI->>Service: loadData(page)
    Service->>API: GET /api/currencies?page=1&size=10
    API-->>Service: PaginatedList<CurrencyRecord>
    Service-->>AdminUI: currencies[] + pagination info
    AdminUI->>AdminUI: Render table

    User->>Dialog: Click "Создать запись"
    Dialog->>User: Show edit form modal
    User->>Dialog: Fill form (currency, value, date)
    User->>Dialog: Submit
    Dialog->>Service: create(CreateCurrencyRecordRequest)
    Service->>API: POST /api/currencies/
    API-->>Service: CurrencyRecord
    Service-->>Dialog: Success
    Dialog->>AdminUI: Close & refresh
    AdminUI->>Service: loadData(currentPage)
    Service->>API: GET /api/currencies?...
    API-->>Service: Updated PaginatedList
    AdminUI->>User: Show updated table + success alert

    User->>AdminUI: Click delete on row
    AdminUI->>Dialog: Open confirm dialog
    User->>Dialog: Confirm deletion
    Dialog->>Service: delete(id)
    Service->>API: DELETE /api/currencies/{id}
    API-->>Service: void (success)
    Service-->>AdminUI: Deletion confirmed
    AdminUI->>Service: loadData(currentPage)
    Service->>API: GET /api/currencies?...
    API-->>Service: Updated PaginatedList
    AdminUI->>User: Refresh table + success alert

sequenceDiagram
    participant User as User
    participant ChartUI as Currencies Chart Page
    participant Service as CurrenciesService
    participant API as Backend API
    participant Chart as Chart.js

    Note over User,Chart: Chart Data Fetch & Render Flow
    
    User->>ChartUI: Navigate to currencies-chart
    ChartUI->>ChartUI: ngOnInit() → isLoading=true
    ChartUI->>Service: getChartData()
    Service->>API: GET /api/currencies/chart
    API-->>Service: CurrencyChartResponse{weeklyData[]}
    Service-->>ChartUI: Observable<CurrencyChartResponse>
    ChartUI->>ChartUI: Store weeklyData, isLoading=false
    ChartUI->>ChartUI: initChart()
    ChartUI->>Chart: new CurrenciesChartObject(weeklyData)
    Chart->>Chart: Build datasets per currency<br/>(excluding KZT)
    Chart->>Chart: Set labels from weekStartDate
    Chart->>Chart: Create Chart.js instance
    Chart-->>ChartUI: chart object
    ChartUI->>User: Render canvas with chart

    User->>ChartUI: Click refresh button
    ChartUI->>Service: getChartData()
    Service->>API: GET /api/currencies/chart
    API-->>Service: Updated CurrencyChartResponse
    Service-->>ChartUI: Observable<CurrencyChartResponse>
    ChartUI->>ChartUI: Update weeklyData
    ChartUI->>Chart: Destroy old chart
    ChartUI->>Chart: Create new CurrenciesChartObject
    Chart-->>ChartUI: Updated chart
    ChartUI->>User: Re-render chart with fresh data

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title 'Added classes' is too vague and generic, failing to convey the substantial feature additions in this pull request.	Replace with a more specific title that captures the main changes, such as 'Add currencies management page with chart visualization' or 'Implement currency records CRUD and weekly chart display'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 6

🧹 Nitpick comments (19)

src/app/modules/admin/components/currencies-page/currencies-page.component.scss (2)
1-7: Consider using word-break: break-word instead.

The word-break: break-all property can break words at any character, which may reduce readability for JSON content. Using break-word would break only at word boundaries when possible, providing better text flow.
🔎 Suggested alternative
 .currencies-json {
   max-height: 400px;
   overflow-y: auto;
   font-size: 0.85rem;
   white-space: pre-wrap;
-  word-break: break-all;
+  word-break: break-word;
 }
9-11: Scope the code selector more specifically.

While Angular's view encapsulation should scope this to the component, using a bare code selector is less explicit. Consider scoping it within .currencies-json code for clarity and to avoid any potential conflicts.
🔎 More specific selector
+.currencies-json code {
-code {
   font-size: 0.8rem;
 }
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-page.component.scss (2)
7-10: Avoid using !important for canvas sizing.

The !important flags suggest a specificity conflict with other styles. This is generally a code smell. Consider using more specific selectors or restructuring the CSS to avoid the need for !important.
🔎 Alternative approach

If you're fighting with chart library styles, try scoping more specifically:
-canvas {
+.chart-container canvas {
-  width: 100% !important;
-  height: 100% !important;
+  width: 100%;
+  height: 100%;
 }
If the chart library applies inline styles, you may need to configure the library options instead of overriding with CSS.
1-5: Consider responsive chart height.

The fixed height of 450px works for desktop, but might not scale well on smaller screens. Consider using viewport-relative units or media queries for better responsiveness.
🔎 Responsive alternative
 .chart-container {
   position: relative;
-  height: 450px;
+  height: min(450px, 60vh);
   width: 100%;
 }
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-page.component.spec.ts (2)

49-56: Consider expanding test coverage when time permits.

The current tests cover basic component creation and data loading, which is a good start for MVP. However, consider adding tests for:

Error handling when chart data fails to load

Chart rendering and initialization

Component cleanup (ngOnDestroy, subscription management)

Different data scenarios (empty data, single data point, etc.)

Based on learnings, I understand you're prioritizing MVP functionality, so this can be deferred.

41-41: CUSTOM_ELEMENTS_SCHEMA might hide component declaration issues.

Using CUSTOM_ELEMENTS_SCHEMA suppresses errors for unrecognized elements, which can mask missing component declarations or imports. If the component uses specific Angular components or directives, consider declaring them explicitly in the test configuration instead.

src/app/modules/admin/components/currencies-page/currencies-page.component.spec.ts (1)

50-52: Consider expanding test coverage for CRUD operations.

The test suite currently only verifies component creation. Consider adding tests for:

Create/delete operations and their success/error flows

Pagination behavior

Form validation in the edit dialog

Alert service integration

Based on learnings, you may choose to defer this to a later phase while focusing on MVP functionality.
src/app/modules/admin/components/currencies-page/currency-record-edit-form.ts (1)
6-14: Consider using typed form controls for better type safety.

The FormControl instances could benefit from explicit generic type parameters to leverage Angular's typed forms feature (available since Angular 14).
🔎 Proposed refactor with typed controls
  constructor() {
    super({
-     currency: new FormControl(CurrencyType.USD, [Validators.required]),
-     value: new FormControl(null, [Validators.required, Validators.min(0.0001)]),
-     pubDate: new FormControl(new Date().toISOString().split("T")[0], [
+     currency: new FormControl<CurrencyType>(CurrencyType.USD, [Validators.required]),
+     value: new FormControl<number | null>(null, [Validators.required, Validators.min(0.0001)]),
+     pubDate: new FormControl<string>(new Date().toISOString().split("T")[0], [
        Validators.required,
      ]),
    });
  }
As per coding guidelines, proper typing improves maintainability and catches type errors at compile time.
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-object.ts (2)

29-39: Consider i18n support for hardcoded Russian text.

Multiple locations contain hardcoded Russian strings:

Currency labels (lines 29-39): "Не указано"

Axis titles (lines 101, 107): "Курс в KZT", "Неделя"

Tooltip suffix (line 130): "KZT"

For better i18n readiness, consider using Angular's i18n framework or extracting these strings to a translatable resource.

As per coding guidelines, templates and code should be i18n ready.

Also applies to: 101-101, 107-107, 130-130

52-54: Document the KZT filtering rationale.

The code explicitly excludes CurrencyType.KZT from the chart without explanation. Consider adding a comment explaining why KZT is filtered (e.g., "KZT is excluded as it's the base currency for comparison").
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-page.component.html (2)
1-1: Template lacks i18n support.

All user-facing text is hardcoded in Russian. Consider using Angular's i18n attributes or translation pipes to make the template localizable.

Example:
<app-page-header i18n>График курсов валют</app-page-header>
As per coding guidelines, templates should be i18n ready.

Also applies to: 8-11, 19-19, 51-51, 68-70

39-49: Add accessibility attributes to decorative SVG.

The SVG icon should include aria-hidden="true" since it's decorative and the adjacent text already conveys the "no data" message.
🔎 Proposed accessibility improvement
        <svg
+         aria-hidden="true"
          xmlns="http://www.w3.org/2000/svg"
          width="64"
          height="64"
As per coding guidelines, ensure proper accessibility with ARIA attributes.
src/app/modules/admin/components/currencies-page/currencies-page.component.html (2)
27-27: Add trackBy function for performance.

The *ngFor loop should use a trackBy function to optimize rendering performance and prevent unnecessary DOM updates when the list refreshes.
🔎 Suggested implementation

In the component TypeScript file, add:
trackByCurrencyId(index: number, item: CurrencyItem): string {
  return item.id;
}
Then update the template:
-<tr *ngFor="let item of currencies" class="small">
+<tr *ngFor="let item of currencies; trackBy: trackByCurrencyId" class="small">
As per coding guidelines for Angular templates.

84-84: Consider trackBy for consistency.

While currencyOptions is a static readonly array, adding a trackBy function improves consistency and is a best practice for all *ngFor loops.
🔎 Suggested implementation

In the component TypeScript file, add:
trackByCurrencyValue(index: number, option: { value: CurrencyType; label: string }): CurrencyType {
  return option.value;
}
Then update the template:
-<option *ngFor="let option of currencyOptions" [ngValue]="option.value">
+<option *ngFor="let option of currencyOptions; trackBy: trackByCurrencyValue" [ngValue]="option.value">
As per coding guidelines for Angular templates.
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-page.component.ts (3)
20-20: Use a more deterministic approach for canvas ID.

Using Math.random() for generating the canvas ID makes testing difficult and is not SSR-friendly. Consider using a static counter or a more predictable approach.
🔎 Suggested implementation
+  private static chartIdCounter = 0;
-  readonly canvasId = "currencies_chart_" + Math.random().toString(36);
+  readonly canvasId = `currencies_chart_${CurrenciesChartPageComponent.chartIdCounter++}`;
As per coding guidelines for Angular components.

65-77: Use @ViewChild instead of document.getElementById.

Direct DOM manipulation via document.getElementById breaks Angular's abstraction and makes testing harder. Use @ViewChild and ElementRef to access the canvas element.
🔎 Suggested implementation
+import { Component, ElementRef, OnDestroy, OnInit, ViewChild } from "@angular/core";

+  @ViewChild('currenciesChart', { static: false }) 
+  canvasRef!: ElementRef<HTMLCanvasElement>;

   private initChart(): void {
     if (!this.chartData || this.chartData.length === 0) {
       return;
     }

     this.chart?.destroy();
     this.chart = new CurrenciesChartObject(this.canvasId, this.chartData);

-    const chartEl = document.getElementById(this.canvasId);
-    if (chartEl != null && chartEl.parentElement != null) {
-      chartEl.style.height = chartEl?.parentElement.style.height ?? "100%";
+    if (this.canvasRef?.nativeElement?.parentElement) {
+      this.canvasRef.nativeElement.style.height = 
+        this.canvasRef.nativeElement.parentElement.style.height || "100%";
     }
   }
In the template, add the template reference variable:
<canvas #currenciesChart [id]="canvasId"></canvas>
As per coding guidelines for Angular components.

56-61: Consider using a logging service instead of console.error.

While console.error works, using a centralized logging service provides better error tracking, monitoring, and consistency across the application.

As per coding guidelines for Angular services and error handling best practices.
src/app/modules/admin/components/currencies-page/currency-item.ts (1)
3-27: Consider using getters instead of copying all fields.

The constructor copies all fields from the item parameter, which creates redundant data storage. You could use getters to expose the properties directly from the stored item.
🔎 Suggested implementation
 export class CurrencyItem implements CurrencyRecord {
+  private readonly _currenciesShort: string;
+  private readonly _currenciesJson: string;
+
   constructor(private readonly item: CurrencyRecord) {
-    this.id = item.id;
-    this.currencyDate = item.currencyDate;
-    this.currencies = item.currencies;
-    this.createdAt = item.createdAt;
-    this.updatedAt = item.updatedAt;
-
     // Create truncated JSON string for display (15 chars)
     const jsonString = JSON.stringify(item.currencies);
-    this.currenciesShort =
+    this._currenciesShort =
       jsonString.length > 15 ? jsonString.substring(0, 15) + "..." : jsonString;
-    this.currenciesJson = jsonString;
+    this._currenciesJson = jsonString;
   }

-  readonly id: string;
-  readonly currencyDate: string;
-  readonly currencies: Record<string, number>;
-  readonly createdAt: string;
-  readonly updatedAt: string;
+  get id(): string { return this.item.id; }
+  get currencyDate(): string { return this.item.currencyDate; }
+  get currencies(): Record<string, number> { return this.item.currencies; }
+  get createdAt(): string { return this.item.createdAt; }
+  get updatedAt(): string { return this.item.updatedAt; }

   // Display helpers
-  readonly currenciesShort: string;
-  readonly currenciesJson: string;
+  get currenciesShort(): string { return this._currenciesShort; }
+  get currenciesJson(): string { return this._currenciesJson; }
 }
As per coding guidelines for TypeScript code organization.
src/app/modules/admin/components/currencies-page/currencies-page.component.ts (1)
17-22: Consider using OnPush change detection strategy.

This component follows a data-flow pattern where state is replaced rather than mutated, making it a good candidate for ChangeDetectionStrategy.OnPush to improve performance.
🔎 Suggested implementation
-import { Component, OnDestroy, OnInit } from "@angular/core";
+import { ChangeDetectionStrategy, Component, OnDestroy, OnInit } from "@angular/core";

 @Component({
   templateUrl: "./currencies-page.component.html",
   styleUrls: ["./currencies-page.component.scss"],
   standalone: false,
+  changeDetection: ChangeDetectionStrategy.OnPush,
 })
Note: You may need to inject ChangeDetectorRef and call markForCheck() after async operations if you encounter view update issues.
As per coding guidelines for Angular components - OnPush change detection where applicable.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1acf9cb and 827a635.

📒 Files selected for processing (15)

src/app/components/navbar/navbar.component.ts
src/app/modules/admin/components/currencies-page/currencies-page.component.html
src/app/modules/admin/components/currencies-page/currencies-page.component.scss
src/app/modules/admin/components/currencies-page/currencies-page.component.spec.ts
src/app/modules/admin/components/currencies-page/currencies-page.component.ts
src/app/modules/admin/components/currencies-page/currency-item.ts
src/app/modules/admin/components/currencies-page/currency-record-edit-form.ts
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-object.ts
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-page.component.html
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-page.component.scss
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-page.component.spec.ts
src/app/modules/salaries/components/currencies-chart-page/currencies-chart-page.component.ts
src/app/modules/salaries/salaries-routing.module.ts
src/app/modules/salaries/salaries.module.ts
src/app/services/currencies-collection.service.ts

🧰 Additional context used

📓 Path-based instructions (6)

**/*.ts