-
-
Notifications
You must be signed in to change notification settings - Fork 18
refactor: migrate cedar policy and group management to Angular signals #1688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
97a90d6
cba862f
3a692f0
7948498
54b9a6e
9069b24
d012a4c
9993894
8482771
e652dd1
4b5dba4
8c4397c
5de6a06
2e79f6e
d7c4ab7
b76ba10
15c7ffb
ac0960a
759336d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import { SelectionModel } from '@angular/cdk/collections'; | ||
| import { CommonModule } from '@angular/common'; | ||
| import { Component, OnDestroy, OnInit } from '@angular/core'; | ||
| import { Component, inject, OnDestroy, OnInit } from '@angular/core'; | ||
| import { MatButtonModule } from '@angular/material/button'; | ||
| import { MatDialog, MatDialogModule } from '@angular/material/dialog'; | ||
| import { MatIconModule } from '@angular/material/icon'; | ||
|
|
@@ -17,6 +17,7 @@ import { ServerError } from 'src/app/models/alert'; | |
| import { CustomEvent, TableProperties } from 'src/app/models/table'; | ||
| import { ConnectionSettingsUI, UiSettings } from 'src/app/models/ui-settings'; | ||
| import { User } from 'src/app/models/user'; | ||
| import { CedarPermissionService } from 'src/app/services/cedar-permission.service'; | ||
| import { CompanyService } from 'src/app/services/company.service'; | ||
| import { ConnectionsService } from 'src/app/services/connections.service'; | ||
| import { TableRowService } from 'src/app/services/table-row.service'; | ||
|
|
@@ -112,7 +113,16 @@ export class DashboardComponent implements OnInit, OnDestroy { | |
| public dialog: MatDialog, | ||
| private title: Title, | ||
| private angulartics2: Angulartics2, | ||
| ) {} | ||
| ) { | ||
| this.canEditConnection = this._permissions.canI( | ||
| 'connection:edit', | ||
| 'Connection', | ||
| this._connections.currentConnectionID, | ||
| ); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| private _permissions = inject(CedarPermissionService); | ||
| protected canEditConnection: ReturnType<CedarPermissionService['canI']>; | ||
|
|
||
| get currentConnectionAccessLevel() { | ||
| return this._connections.currentConnectionAccessLevel; | ||
|
|
@@ -159,66 +169,68 @@ export class DashboardComponent implements OnInit, OnDestroy { | |
| getData() { | ||
| console.log('getData'); | ||
|
|
||
| this._tables.fetchTablesFolders(this.connectionID).subscribe((res) => { | ||
| const tables = res.find((item) => item.category_id === 'all-tables-kitten')?.tables || []; | ||
|
|
||
| this.tableFolders = res; | ||
|
|
||
| if (tables && tables.length === 0) { | ||
| this.noTablesError = true; | ||
| this.loading = false; | ||
| this.title.setTitle(`No tables | ${this._company.companyTabTitle || 'Rocketadmin'}`); | ||
| } else if (tables) { | ||
| this.formatTableNames(); | ||
| this.route.paramMap | ||
| .pipe( | ||
| map((params: ParamMap) => { | ||
| let tableName = params.get('table-name'); | ||
| if (tableName) { | ||
| this.selectedTableName = tableName; | ||
| this.setTable(tableName); | ||
| console.log('setTable from getData paramMap'); | ||
| this.title.setTitle( | ||
| `${this.selectedTableDisplayName} table | ${this._company.companyTabTitle || 'Rocketadmin'}`, | ||
| ); | ||
| this.selection.clear(); | ||
| } else { | ||
| if (this.defaultTableToOpen) { | ||
| tableName = this.defaultTableToOpen; | ||
| this._tables.fetchTablesFolders(this.connectionID).subscribe( | ||
| (res) => { | ||
| const tables = res.find((item) => item.category_id === 'all-tables-kitten')?.tables || []; | ||
|
|
||
| this.tableFolders = res; | ||
|
|
||
| if (tables && tables.length === 0) { | ||
| this.noTablesError = true; | ||
| this.loading = false; | ||
| this.title.setTitle(`No tables | ${this._company.companyTabTitle || 'Rocketadmin'}`); | ||
| } else if (tables) { | ||
| this.formatTableNames(); | ||
| this.route.paramMap | ||
| .pipe( | ||
| map((params: ParamMap) => { | ||
| let tableName = params.get('table-name'); | ||
| if (tableName) { | ||
| this.selectedTableName = tableName; | ||
| this.setTable(tableName); | ||
| console.log('setTable from getData paramMap'); | ||
| this.title.setTitle( | ||
| `${this.selectedTableDisplayName} table | ${this._company.companyTabTitle || 'Rocketadmin'}`, | ||
| ); | ||
| this.selection.clear(); | ||
| } else { | ||
| tableName = this.allTables[0]?.table; | ||
| if (this.defaultTableToOpen) { | ||
| tableName = this.defaultTableToOpen; | ||
| } else { | ||
| tableName = this.allTables[0]?.table; | ||
| } | ||
| this.router.navigate([`/dashboard/${this.connectionID}/${tableName}`], { replaceUrl: true }); | ||
| this.selectedTableName = tableName; | ||
| } | ||
| this.router.navigate([`/dashboard/${this.connectionID}/${tableName}`], { replaceUrl: true }); | ||
| this.selectedTableName = tableName; | ||
| } | ||
| }), | ||
| ) | ||
| .subscribe(); | ||
| this._tableRow.cast.subscribe((arg) => { | ||
| if (arg === 'delete row' && this.selectedTableName) { | ||
| this.setTable(this.selectedTableName); | ||
| console.log('setTable from getData _tableRow cast'); | ||
| this.selection.clear(); | ||
| } | ||
| }); | ||
| this._tables.cast.subscribe((arg) => { | ||
| if ((arg === 'delete rows' || arg === 'import') && this.selectedTableName) { | ||
| this.setTable(this.selectedTableName); | ||
| console.log('setTable from getData _tables cast'); | ||
| this.selection.clear(); | ||
| } | ||
| if (arg === 'activate actions') { | ||
| this.selection.clear(); | ||
| } | ||
| }); | ||
| } | ||
| }, | ||
| (err) => { | ||
| this.isServerError = true; | ||
| this.serverError = { abstract: err.error?.message || err.message, details: err.error?.originalMessage }; | ||
| this.loading = false; | ||
| this.title.setTitle(`Error | ${this._company.companyTabTitle || 'Rocketadmin'}`); | ||
| }); | ||
| }), | ||
| ) | ||
| .subscribe(); | ||
| this._tableRow.cast.subscribe((arg) => { | ||
| if (arg === 'delete row' && this.selectedTableName) { | ||
| this.setTable(this.selectedTableName); | ||
| console.log('setTable from getData _tableRow cast'); | ||
| this.selection.clear(); | ||
| } | ||
| }); | ||
| this._tables.cast.subscribe((arg) => { | ||
| if ((arg === 'delete rows' || arg === 'import') && this.selectedTableName) { | ||
| this.setTable(this.selectedTableName); | ||
| console.log('setTable from getData _tables cast'); | ||
| this.selection.clear(); | ||
| } | ||
| if (arg === 'activate actions') { | ||
| this.selection.clear(); | ||
| } | ||
| }); | ||
|
Comment on lines
+183
to
+223
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Each successful fetch adds fresh subscriptions to Based on learnings, 🤖 Prompt for AI Agents |
||
| } | ||
| }, | ||
| (err) => { | ||
| this.isServerError = true; | ||
| this.serverError = { abstract: err.error?.message || err.message, details: err.error?.originalMessage }; | ||
| this.loading = false; | ||
| this.title.setTitle(`Error | ${this._company.companyTabTitle || 'Rocketadmin'}`); | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| formatTableNames() { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Dependency injection and signal declarations are misplaced in class structure.
The
_permissionsinjection andcanEditConnectiondeclaration are placed in the middle of the class (after the constructor, before methods), violating the component structure guideline. Move these to the top of the class, right after the existing property declarations.♻️ Suggested reorganization
export class ConnectionSettingsComponent implements OnInit { protected posthog = posthog; public isSaas = (environment as any).saas; + private _permissions = inject(CedarPermissionService); + protected canEditConnection: ReturnType<CedarPermissionService['canI']>; public connectionID: string | null = null; // ... other properties ... - private _permissions = inject(CedarPermissionService); - protected canEditConnection: ReturnType<CedarPermissionService['canI']>; getSettings() { // ... }As per coding guidelines: "Structure components with dependency injection at the top, followed by signals, computed signals, effects, lifecycle hooks, public methods, then private methods"
🤖 Prompt for AI Agents