Added edit feature #76
Conversation
WalkthroughThe changes introduce inline editing for company details on the admin company page, including a modal form and an actions dropdown. The company update service is implemented, and request types are unified under Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant Page as CompanyAdminPageComponent
participant Modal as EditCompanyForm Modal
participant Service as CompaniesService
participant API as Backend API
Admin->>Page: Clicks "Actions" dropdown
Admin->>Page: Selects "Редактировать"
Page->>Modal: Opens EditCompanyForm with current company data
Admin->>Modal: Edits and submits form
Modal->>Page: Calls onEditFormSubmit()
Page->>Service: update(companyId, editRequest)
Service->>API: POST /api/companies/{id} with editRequest
API-->>Service: Success response
Service-->>Page: Observable completes
Page->>Modal: Closes modal
Page->>Page: Reloads company data
Page->>Admin: Shows success alert
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Backend: Techinterview-space/web-api#105 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/app/modules/admin/components/companies/company-admin-page/company-admin-page.component.html (1)
248-287: Edit form modal implementation is missing a cancel buttonThe edit form modal looks good, but consider adding a cancel button next to the save button to provide users with a clear way to discard their changes.
<div class="mb-3"> <button class="btn btn-primary" type="submit">Сохранить</button> + <button type="button" class="btn btn-secondary ms-2" (click)="onEditModalDlgClose()">Отмена</button> </div>src/app/modules/admin/components/companies/company-admin-page/company-admin-page.component.ts (1)
72-90: Form submission looks good but consider refactoring data loadingThe form submission logic is well-structured with proper validation checks. However, calling
ngOnInit()directly to refresh data is not a typical Angular pattern.Consider extracting the data loading logic to a separate method that can be called from both
ngOnInit()and after form submission:onEditFormSubmit(): void { if (this.editForm == null || this.company == null) { return; } const editRequest = this.editForm.editRequestOrNull(); if (editRequest == null) { return; } this.service .update(this.company.id, editRequest) .pipe(untilDestroyed(this)) .subscribe(() => { this.alertService.success("Компания была обновлена"); this.editForm = null; - this.ngOnInit(); + this.loadCompanyData(); }); } + private loadCompanyData(): void { + this.activateRoute + .getParam("id") + .pipe(untilDestroyed(this)) + .subscribe((id) => { + this.service + .byId(id!) + .pipe(untilDestroyed(this)) + .subscribe((i) => { + this.company = i; + this.company!.description = this.company!.description?.replace( + /\n/g, + "<br />", + ); + + this.company!.reviews.forEach((r) => { + r.pros = r.pros?.replace(/\n/g, "<br />"); + r.cons = r.cons?.replace(/\n/g, "<br />"); + }); + + this.title.setTitle(`Компания ${this.company!.name}`); + }); + }); + }Then update
ngOnInit()to call this method:ngOnInit(): void { - this.activateRoute - .getParam("id") - .pipe(untilDestroyed(this)) - .subscribe((id) => { - this.service - .byId(id!) - .pipe(untilDestroyed(this)) - .subscribe((i) => { - this.company = i; - this.company!.description = this.company!.description?.replace( - /\n/g, - "<br />", - ); - - this.company!.reviews.forEach((r) => { - r.pros = r.pros?.replace(/\n/g, "<br />"); - r.cons = r.cons?.replace(/\n/g, "<br />"); - }); - - this.title.setTitle(`Компания ${this.company!.name}`); - }); - }); + this.loadCompanyData(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/app/modules/admin/components/companies/companies-admin-page/companies-admin-page.component.ts(1 hunks)src/app/modules/admin/components/companies/company-admin-page/company-admin-page.component.html(2 hunks)src/app/modules/admin/components/companies/company-admin-page/company-admin-page.component.ts(3 hunks)src/app/modules/admin/components/companies/shared/edit-company-form.ts(2 hunks)src/app/modules/admin/components/telegram/inline-replies-stats/inline-replies-stats.component.html(1 hunks)src/app/services/companies.service.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/app/modules/admin/components/companies/shared/edit-company-form.ts (1)
src/app/services/companies.service.ts (1)
CompanyEditRequest(17-22)
src/app/modules/admin/components/companies/company-admin-page/company-admin-page.component.ts (1)
src/app/modules/admin/components/companies/shared/edit-company-form.ts (1)
EditCompanyForm(5-42)
🔇 Additional comments (11)
src/app/modules/admin/components/telegram/inline-replies-stats/inline-replies-stats.component.html (1)
1-1: Updated header text for clarity.The header has been changed from “Использование бота” to “График использования бота” to better convey that this view displays a usage chart.
src/app/modules/admin/components/companies/companies-admin-page/companies-admin-page.component.ts (2)
4-4: Import optimization looks goodReplaced the unused
PageParamsimport with the more specificdefaultPageParamsthat's actually used in the code.
9-9: Path update for shared component is correctThe import path for
EditCompanyFormhas been properly updated to point to the shared location.src/app/modules/admin/components/companies/shared/edit-company-form.ts (2)
3-3: Interface import update is correctUpdated import to use the renamed
CompanyEditRequestinterface.
23-23: Return type update is consistentUpdated return type to match the renamed interface
CompanyEditRequest.src/app/services/companies.service.ts (2)
17-22: Interface renaming is a good practiceRenaming
CompanyCreateRequesttoCompanyEditRequestunifies the request type for both create and update operations, making the code more consistent.
53-53: Parameter type update is consistentThe parameter type has been correctly updated to use the renamed interface.
src/app/modules/admin/components/companies/company-admin-page/company-admin-page.component.ts (4)
92-94: Implement or hide unfinished functionalityThe
recalculateRating()method is a stub that throws an error. This suggests there's a UI element that triggers this method, which would result in an error for users.Please verify that this method is not accessible via the UI, or implement the functionality properly. If this feature is planned for a future update, consider either:
- Implementing a placeholder that shows a "Coming soon" message to users
- Disabling the UI element that triggers this method
- Hiding the UI element completely until the functionality is implemented
recalculateRating(): void { - throw new Error("Not implemented"); + this.alertService.info("Функция пересчета рейтинга будет доступна в ближайшем обновлении"); }
11-11: LGTM! Proper import for EditCompanyForm.The import statement for the EditCompanyForm is correct and follows the project's relative path convention.
22-22: LGTM! Property is properly typed as nullable.The
editFormproperty is correctly typed as nullableEditCompanyForm | nullwith proper initialization tonull.
68-70: LGTM! Clear method for closing modal.The
onEditModalDlgClose()method correctly handles clearing the form reference when the modal is closed.
| <div class="d-flex justify-content-end"> | ||
| <div class="btn-group" role="group"> | ||
| <button | ||
| type="button" | ||
| class="btn btn-outline-dark dropdown-toggle" | ||
| data-bs-toggle="dropdown" | ||
| aria-expanded="false" | ||
| > | ||
| Действия | ||
| </button> | ||
| <ul class="dropdown-menu"> | ||
| <li> | ||
| <button | ||
| type="button" | ||
| class="dropdown-item" | ||
| (click)="openEditForm()" | ||
| > | ||
| Редактировать | ||
| </button> | ||
| </li> | ||
| <li> | ||
| <button | ||
| type="button" | ||
| class="dropdown-item" | ||
| (click)="recalculateRating()" | ||
| > | ||
| Пересчитать рейтинг | ||
| </button> | ||
| </li> | ||
| <li> | ||
| <a | ||
| routerLink="/companies/{{ company.id }}" | ||
| class="dropdown-item" | ||
| >Публичная страница</a | ||
| > | ||
| </li> | ||
| </ul> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Actions dropdown menu is well structured
The dropdown menu provides good access to company actions. However, I have concerns about the recalculate rating functionality.
🏁 Script executed:
#!/bin/bash
# Check if recalculateRating() is implemented properly or throws a not implemented error
rg -A 3 "recalculateRating\(\)" --type tsLength of output: 527
Implement or remove the “Пересчитать рейтинг” action
The recalculateRating() method currently throws a Not implemented error, so clicking “Пересчитать рейтинг” will break the UI. Please implement the actual rating–recalculation logic or disable/remove this menu item until it’s ready.
Affected location:
- src/app/modules/admin/components/companies/company-admin-page/company-admin-page.component.ts
recalculateRating(): void { throw new Error("Not implemented"); }
Suggested fix (example):
-recalculateRating(): void {
- throw new Error("Not implemented");
-}
+recalculateRating(): void {
+ // TODO: replace with real implementation, e.g.:
+ this.companyService.recalculateRating(this.company.id)
+ .subscribe(updated => (this.company = updated));
+}| update(id: string, model: CompanyEditRequest): Observable<void> { | ||
| return this.api.post(this.apiUrl + id, model); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider using PUT instead of POST for updates
While the implementation works, using POST for updates deviates from RESTful conventions. Consider using PUT or PATCH which are more semantically correct for update operations.
update(id: string, model: CompanyEditRequest): Observable<void> {
- return this.api.post(this.apiUrl + id, model);
+ return this.api.put(this.apiUrl + id, model);
}🏁 Script executed:
#!/bin/bash
# Check if ApiService supports PUT requests
rg -A 1 "put\(" --type ts src/app/services/api.service.tsLength of output: 59
To ensure whether ApiService actually exposes a put (or patch) method, let’s locate its definition and list supported HTTP verbs:
🏁 Script executed:
#!/bin/bash
# Locate the ApiService file
API_FILE=$(fd --type f 'api.service.ts' | head -n 1)
echo "ApiService file: $API_FILE"
# List any HTTP method wrappers in ApiService
grep -E 'post|get|put|patch' -n "$API_FILE" || echo "No HTTP methods found matching post|get|put|patch"Length of output: 670
Use HTTP PUT for update operations
The ApiService already exposes a put method (see src/app/services/api.service.ts:40–41), so switching to PUT will align with RESTful conventions for full updates.
• File: src/app/services/companies.service.ts
• Lines: 49–51
update(id: string, model: CompanyEditRequest): Observable<void> {
- return this.api.post(this.apiUrl + id, model);
+ return this.api.put(this.apiUrl + id, model);
}If you ever need partial updates, you can similarly use this.api.patch(...).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| update(id: string, model: CompanyEditRequest): Observable<void> { | |
| return this.api.post(this.apiUrl + id, model); | |
| } | |
| update(id: string, model: CompanyEditRequest): Observable<void> { | |
| return this.api.put(this.apiUrl + id, model); | |
| } |
| openEditForm(): void { | ||
| this.editForm = new EditCompanyForm(this.company); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check before creating form
The openEditForm() method should check if this.company is null before creating the form to prevent potential runtime errors.
openEditForm(): void {
+ if (this.company === null) {
+ return;
+ }
this.editForm = new EditCompanyForm(this.company);
}
Summary by CodeRabbit
New Features
Improvements
Style