-
Notifications
You must be signed in to change notification settings - Fork 4
[NAE-2227] MenuItem default headers not working #304
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 all commits
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 |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ function navigationItemDefaultHeadersFactory(navigationItemTaskData: DataGroup[] | |
| return undefined; | ||
| } | ||
| } catch (e) { | ||
| console.log('Error during resolving default headers', e) | ||
|
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. 🛠️ Refactor suggestion | 🟠 Major Use LoggerService instead of console.log. Direct console logging bypasses the application's logging infrastructure and may not be captured in production monitoring systems. Consider injecting and using LoggerService: - console.log('Error during resolving default headers', e)
+ // Inject LoggerService as a parameter if not already available
+ loggerService.error('Error during resolving default headers', e);Alternatively, if LoggerService cannot be injected at the factory level, consider propagating the error instead of swallowing it: - } catch (e) {
- console.log('Error during resolving default headers', e)
- return undefined;
- }
+ } catch (e) {
+ throw new Error(`Error during resolving default headers: ${e}`);
+ }
🤖 Prompt for AI Agents |
||
| return undefined; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import {AfterViewInit, Component, Inject, ViewChild} from '@angular/core'; | ||
| import {AfterViewInit, Component, Inject, Optional, ViewChild} from '@angular/core'; | ||
| import { | ||
| AbstractTabbedCaseViewComponent, | ||
| AllowedNetsService, | ||
|
|
@@ -85,12 +85,14 @@ export class DefaultTabbedCaseViewComponent extends AbstractTabbedCaseViewCompon | |
| headersMode: string[]; | ||
| allowTableMode: boolean; | ||
| defaultHeadersMode: HeaderMode; | ||
| headersCount: number | ||
|
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. 🧹 Nitpick | 🔵 Trivial Consider initializing headersCount at declaration. While Apply this diff: - headersCount: number
+ headersCount: number = 5;Then in the constructor, you can simplify: - if (this._defaultHeaders) {
- this.headersCount = this._defaultHeaders.length;
- } else {
- this.headersCount = 5; // 5 meta headers
- }
+ if (this._defaultHeaders) {
+ this.headersCount = this._defaultHeaders.length;
+ }🤖 Prompt for AI Agents |
||
|
|
||
| constructor(caseViewService: CaseViewService, | ||
| loggerService: LoggerService, | ||
| viewIdService: ViewIdService, | ||
| overflowService: OverflowService, | ||
| @Inject(NAE_TAB_DATA) protected _injectedTabData: InjectedTabbedCaseViewDataWithNavigationItemTaskData) { | ||
| @Inject(NAE_TAB_DATA) protected _injectedTabData: InjectedTabbedCaseViewDataWithNavigationItemTaskData, | ||
| @Optional() @Inject(NAE_DEFAULT_HEADERS) protected _defaultHeaders: Array<string> | undefined) { | ||
| super(caseViewService, loggerService, _injectedTabData, overflowService, undefined, undefined, _injectedTabData.newCaseButtonConfiguration); | ||
|
|
||
| this.initialSearchMode = _injectedTabData.caseViewSearchTypeConfiguration.initialSearchMode; | ||
|
|
@@ -107,6 +109,11 @@ export class DefaultTabbedCaseViewComponent extends AbstractTabbedCaseViewCompon | |
| const viewId = viewIdService.viewId; | ||
| localStorage.setItem(viewId + '-overflowMode', 'false'); | ||
| } | ||
| if (this._defaultHeaders) { | ||
| this.headersCount = this._defaultHeaders.length; | ||
| } else { | ||
| this.headersCount = 5; // 5 meta headers | ||
| } | ||
| } | ||
|
|
||
| ngAfterViewInit(): void { | ||
|
|
||
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.
🧩 Analysis chain
Breaking change: verify all usages and backend field names.
The constant value changed from
'use_default_headers'to'use_case_default_headers'. This is a breaking change that will affect:Additionally, there's an inconsistency:
ITEM_FIELD_ID_USE_CASE_DEFAULT_HEADERSnow uses'use_case_default_headers'(line 126), butITEM_FIELD_ID_USE_TASK_DEFAULT_HEADERSstill uses'use_default_headers'(line 136). If this inconsistency is intentional, consider adding a comment explaining why; otherwise, align both constants.Run the following script to check for any remaining references to the old field name:
🏁 Script executed:
Length of output: 0
Run the following to check for any remaining references to the old and new field IDs across the repository:
If neither command yields hits, manually verify any external configurations or backend mappings still expecting
use_default_headers.🏁 Script executed:
Length of output: 8212
Fix and align default‐headers constants
ITEM_FIELD_ID_USE_TASK_DEFAULT_HEADERS(line 136) from'use_default_headers'to'use_task_default_headers'.ITEM_FIELD_ID_USE_CASE_DEFAULT_HEADERSto'use_case_default_headers'doesn’t break existing backend/data‐model mappings or client configurations.