Portfolio summary implementation - #79
Open
kflemin wants to merge 22 commits into
Open
Conversation
…ry-merge # Conflicts: # cspell.json # package.json # pnpm-lock.yaml # src/app/modules/organizations/settings/salesforce-building-integration/salesforce-building-integration.component.ts
- Resolve merge conflicts with main (cspell.json, package.json,
pnpm-lock.yaml, salesforce-building-integration component) by keeping
main's newer Salesforce Building Integration implementation (BB
Salesforce support, barrel imports) under the PR's renamed path.
- Convert new API/component imports to the @seed/api and MaterialImports
barrels per project convention; register goal and salesforce-portfolio
in the @seed/api barrel index.
- Fix missing salesforce-login/salesforce-portfolio-integration .scss
files referenced by styleUrl (never committed) by dropping the
styleUrl instead.
- Add full Transloco translation coverage for all new UI strings across
the portfolio summary, configure-goals, add-cycle,
salesforce-portfolio-integration and salesforce-login templates.
- Fix several crashes found via live testing against a real backend:
- ConfigureGoalsDialogComponent.save() threw when Salesforce
integration was not enabled / no partner was selected.
- AddCycleDialogComponent.submit() threw when no annual report was
selected (Salesforce not enabled).
- PortfolioSummaryComponent.createChart() threw when a goal had no
weighted EUI data yet.
- PortfolioSummaryComponent.openAddCycle() threw because cycle_goals
was never initialized/fetched.
- Removed a stray {{ chart }} interpolation inside the results canvas
that rendered "[object Object]".
- Add GoalService.getCycleGoals() and call it when selecting/updating a
goal so previously created cycle goals load after a refresh (the
Goal API response never embeds cycle_goals).
- Remove leftover console.log debug statements.
Verified with pnpm lint / pnpm build, and by standing up a throwaway
Django backend (docker) with seeded data and exercising the goal
create/edit, add-cycle, and Salesforce settings flows via Playwright.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ry-merge # Conflicts: # public/i18n/en_US.json
- Remove an erroneous p-80 (320px) padding class on the CycleGoal side summary table, which was causing huge, uneven whitespace around that card. Drop the redundant duplicate p-10 alongside p-4 on the goal summary card for the same reason. - Add px-3 py-1 cell padding to the summary table so label/value columns aren't flush against each other. - Wrap the page content in the same content mx-5 my-2 container used by sibling insights pages (Program Overview, Property Insights, Default Reports) instead of leaving it unpadded/flush with the page edge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the Insights “Portfolio Summary” feature area, including goal configuration workflows, Salesforce portfolio sync UI/service plumbing, and related inventory-grid/action enhancements.
Changes:
- Added a full Portfolio Summary page (goal selection, cycle comparison, goal notes editing, exports) plus supporting dialogs (configure goals, add/edit cycle, bulk edit notes, sync-to-Salesforce review).
- Introduced new API clients/types for Goals and Salesforce Portfolio, and extended existing services (labels, inventory, audit-template, bb-salesforce config handling).
- Updated org settings routing/navigation for the Salesforce building integration page; added an OAuth callback route/component for Salesforce login.
Reviewed changes
Copilot reviewed 59 out of 64 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/modules/salesforce-login/salesforce-login.component.ts | New Salesforce OAuth callback component to exchange code for token and redirect. |
| src/app/modules/salesforce-login/salesforce-login.component.spec.ts | Adds a basic unit test for the Salesforce login callback component. |
| src/app/modules/salesforce-login/salesforce-login.component.html | Adds a translated “Logging in…” loading message. |
| src/app/modules/organizations/settings/settings.routes.ts | Renames Salesforce settings route to salesforce-building-integration and switches component. |
| src/app/modules/organizations/settings/settings.component.ts | Updates settings nav link to new Salesforce settings route. |
| src/app/modules/organizations/settings/salesforce-building-integration/salesforce-building-integration.component.ts | Renames/refactors Salesforce settings component and adjusts bb-salesforce config save behavior. |
| src/app/modules/organizations/settings/salesforce-building-integration/salesforce-building-integration.component.html | Minor formatting update to seed-page config binding. |
| src/app/modules/organizations/settings/salesforce-building-integration/modal/index.ts | Barrel export for Salesforce mapping modals. |
| src/app/modules/organizations/settings/salesforce-building-integration/modal/form-modal.component.ts | Adds create/edit Salesforce field mapping modal. |
| src/app/modules/organizations/settings/salesforce-building-integration/modal/form-modal.component.html | Template for Salesforce mapping create/edit modal. |
| src/app/modules/organizations/settings/salesforce-building-integration/modal/delete-modal.component.ts | Adds delete-confirm modal for Salesforce field mappings. |
| src/app/modules/organizations/settings/salesforce-building-integration/modal/delete-modal.component.html | Template for Salesforce mapping delete confirmation modal. |
| src/app/modules/inventory-list/summary/summary.component.ts | Formatting-only refactors in requestColumns and filter matcher logic. |
| src/app/modules/inventory-list/list/inventory.component.html | Passes cycles into grid actions component. |
| src/app/modules/inventory-list/list/grid/label-header.component.ts | New ag-grid header component for toggling label column expansion. |
| src/app/modules/inventory-list/list/grid/grid.component.ts | Adds label column expand/collapse behavior and new compact label rendering. |
| src/app/modules/inventory-list/list/grid/grid.component.html | Provides ag-grid context to support header-driven label toggling. |
| src/app/modules/inventory-list/list/grid/actions.component.ts | Adds audit-template export/import modals and Salesforce update action wiring. |
| src/app/modules/inventory-list/list/grid/actions.component.html | Enables Audit Template and Salesforce menu items conditionally and wires actions. |
| src/app/modules/inventory-list/list/actions/index.ts | Exports newly added audit-template action modals. |
| src/app/modules/inventory-list/list/actions/audit-template-import-modal.component.ts | New modal to import audit-template submissions into inventory with progress tracking. |
| src/app/modules/inventory-list/list/actions/audit-template-import-modal.component.html | Template for audit-template import modal. |
| src/app/modules/inventory-list/list/actions/audit-template-export-modal.component.ts | New modal to export selected properties to Audit Template with progress tracking. |
| src/app/modules/inventory-list/list/actions/audit-template-export-modal.component.html | Template for audit-template export modal. |
| src/app/modules/insights/portfolio-summary/sync-salesforce-dialog/sync-salesforce-dialog.component.ts | New dialog to compare SEED vs Salesforce and sync current/historical reports. |
| src/app/modules/insights/portfolio-summary/sync-salesforce-dialog/sync-salesforce-dialog.component.spec.ts | Comprehensive unit tests for sync-salesforce dialog logic. |
| src/app/modules/insights/portfolio-summary/sync-salesforce-dialog/sync-salesforce-dialog.component.html | Sync-to-Salesforce dialog UI (loading/error states, comparison tables, actions). |
| src/app/modules/insights/portfolio-summary/sync-salesforce-dialog/index.ts | Barrel export for sync-salesforce dialog. |
| src/app/modules/insights/portfolio-summary/portfolio-summary.types.ts | Adds Portfolio Summary dialog/context types. |
| src/app/modules/insights/portfolio-summary/portfolio-summary.component.spec.ts | Adds extensive unit test coverage for Portfolio Summary component logic. |
| src/app/modules/insights/portfolio-summary/portfolio-summary.component.html | Implements the full Portfolio Summary page UI and interactions. |
| src/app/modules/insights/portfolio-summary/portfolio-summary-label-header.component.ts | New label header for Portfolio Summary grids (expand/collapse per column). |
| src/app/modules/insights/portfolio-summary/portfolio-summary-header-menu.component.ts | New ag-grid header menu component using CDK Overlay for sort/pin/hide. |
| src/app/modules/insights/portfolio-summary/portfolio-summary-header-menu.component.html | Template for Portfolio Summary header menu overlay. |
| src/app/modules/insights/portfolio-summary/index.ts | Re-exports Portfolio Summary types in addition to component. |
| src/app/modules/insights/portfolio-summary/configure-goals-dialog/index.ts | Barrel export for configure-goals dialog. |
| src/app/modules/insights/portfolio-summary/configure-goals-dialog/configure-goals-dialog.component.ts | Adds goal creation/editing/deletion dialog (incl. Salesforce partner/goal association). |
| src/app/modules/insights/portfolio-summary/configure-goals-dialog/configure-goals-dialog.component.html | Template for goal configuration dialog. |
| src/app/modules/insights/portfolio-summary/bulk-edit-goal-notes-modal.component.ts | Adds bulk edit modal for goal notes fields across selected properties. |
| src/app/modules/insights/portfolio-summary/bulk-edit-goal-notes-modal.component.html | Template for bulk edit goal notes modal. |
| src/app/modules/insights/portfolio-summary/add-cycle-dialog/index.ts | Barrel export for add-cycle dialog. |
| src/app/modules/insights/portfolio-summary/add-cycle-dialog/add-cycle-dialog.component.ts | Adds add/edit cycle dialog (incl. optional Salesforce annual report association). |
| src/app/modules/insights/portfolio-summary/add-cycle-dialog/add-cycle-dialog.component.html | Template for add/edit cycle dialog. |
| src/app/modules/insights/facilities-plan/facilities-plan.component.spec.ts | Formatting-only changes in tests. |
| src/app/modules/datasets/pairing/pairing.component.ts | Formatting-only change to drag column definition. |
| src/app/app.routes.ts | Adds /salesforce-login route for Salesforce OAuth callback. |
| src/@seed/api/salesforce/salesforce.types.ts | Changes bb_salesforce_configs response shape type. |
| src/@seed/api/salesforce/bb-salesforce.service.ts | Updates bb-salesforce config parsing and update signature to match new API behavior. |
| src/@seed/api/salesforce-portfolio/salesforce-portfolio.types.ts | New types for Salesforce portfolio config/token/partners/annual reports endpoints. |
| src/@seed/api/salesforce-portfolio/salesforce-portfolio.service.ts | New service for Salesforce portfolio endpoints (login/token/partners/annual reports). |
| src/@seed/api/salesforce-portfolio/index.ts | Barrel export for Salesforce portfolio API package. |
| src/@seed/api/label/label.types.ts | Adds PropertyViewLabel type. |
| src/@seed/api/label/label.service.ts | Adds listByCycleGoal call for property view labels. |
| src/@seed/api/inventory/inventory.service.ts | Adds updateSalesforce endpoint wrapper for inventory updates. |
| src/@seed/api/index.ts | Exports new goal and salesforce-portfolio API packages. |
| src/@seed/api/goal/index.ts | Barrel export for new goal API package. |
| src/@seed/api/goal/goal.types.ts | Adds goal/cycle-goal/portfolio-summary/notes/salesforce-summary type definitions. |
| src/@seed/api/goal/goal.service.ts | Adds goal CRUD + portfolio summary + property list + Salesforce sync service methods. |
| src/@seed/api/audit-template/audit-template.service.ts | Adds batch export/import endpoints returning progress_key. |
| public/i18n/fr_CA.json | Adds translations for new Portfolio Summary and related UI strings. |
| public/i18n/es.json | Adds translations for new Portfolio Summary and related UI strings. |
| public/i18n/en_US.json | Adds new keys for Portfolio Summary UI; removes some existing keys. |
| .spelling.dic | Adds domain terms (e.g., kbtu, euis) to spelling dictionary. |
Suppressed comments (5)
src/app/modules/salesforce-login/salesforce-login.component.ts:26
- ngOnInit unconditionally calls getToken with params.code cast to string. If the callback is hit without a code (or with an error param), this will call the API with an invalid value. Guard against missing code and prefer org_id when available.
src/@seed/api/salesforce-portfolio/salesforce-portfolio.service.ts:75 - getAnnualReports is typed as getPartnersResponse (SalesforcePartner[]), but the endpoint is for Annual Reports. This will cause type errors at call sites (and can mask real response-shape issues). Use getAnnualReportsResponse for both the return type and HttpClient generic (and consider a more specific error message).
getAnnualReports(goalId: number): Observable<getPartnersResponse> {
const url = `/api/v3/bb_salesforce/annual_report/?organization_id=${this.orgId}&goal_id=${goalId}`
return this._httpClient.get<getPartnersResponse>(url).pipe(
map((response) => response),
catchError((error: HttpErrorResponse) => {
return this._errorService.handleError(error, `Error fetching summary: ${error.message}`)
}),
)
src/@seed/api/goal/goal.service.ts:80
- editedGoal is implicitly typed as any here. With noImplicitAny enabled this will fail to compile; even without it, it defeats type safety for payload construction. Type it as a Partial (or a narrower payload type) to keep API calls typed.
editGoal(goalId: number, editedGoal, orgId: number): Observable<Goal> {
src/@seed/api/goal/goal.service.ts:89
- newGoal is implicitly typed as any here. With noImplicitAny enabled this will fail to compile; type it as Partial (or a dedicated create payload type) to keep this call type-safe.
createGoal(newGoal, orgId: number): Observable<Goal> {
public/i18n/en_US.json:74
- Several i18n keys were removed from en_US.json but are still referenced in code (e.g., inventory-create.component.ts translates 'Access Level Instance is required', 'Cycle is required', and 'Duplicate columns are not allowed'). Removing these keys will cause missing-translation behavior in the UI. Please restore the removed keys (or update the callers to use the new keys).
"Access Level": "Access Level",
"Access Level Instance": "Access Level Instance",
"Access Level Instance Information": "Access Level Instance Information",
"Access Level Instance Tree": "Access Level Instance Tree",
"Access Level Instance:": "Access Level Instance:",
"Access Level Instances": "Access Level Instances",
"Access Level Instances Errors": "Access Level Instances Errors",
"Access Level Tree": "Access Level Tree",
Comment on lines
+16
to
+24
| const LABEL_COLOR_MAP: Record<string, string> = { | ||
| red: '#b91c1c', | ||
| orange: '#fb923c', | ||
| gray: '#57534e', | ||
| green: '#15803d', | ||
| blue: '#1d4ed8', | ||
| 'light blue': '#0891b2', | ||
| white: '#e5e7eb', | ||
| } |
Contributor
There was a problem hiding this comment.
The fix removes the LABEL_COLOR_MAP hard-coded hex colors and replaces them with CSS class-based coloring consistent with styles.scss .label theme tokens — the same approach used in the expanded label view. The change uses span.className = \label ${colorClass}`` so colors come from the theme and work correctly in both light and dark mode.
The commit is ready locally but the push is being blocked by a repository rule (GH013: Cannot update this protected ref). The workflow runs show action_required (pending approval), which appears to be the blocker. Once that's resolved, the commit can be pushed.
Comment on lines
272
to
277
| .map((id: number) => { | ||
| return this.labelMap[id] | ||
| ? `<div class="label ${this.labelMap[id]?.color} whitespace-nowrap px-2">${this.labelMap[id].name}</div>` | ||
| : '' | ||
| const label = this.labelMap[id] | ||
| if (!label) return '' | ||
| const bg = LABEL_COLOR_MAP[label.color] ?? LABEL_COLOR_MAP.gray | ||
| return `<span style="flex:1;min-width:0;background-color:${bg};" title="${label.name}"></span>` | ||
| }) |
Contributor
There was a problem hiding this comment.
Fixed in the latest commit by replacing the HTML string interpolation with DOM node creation — span.title = label.name sets the property directly, avoiding any attribute injection risk.
Comment on lines
+37
to
+50
| ngAfterViewInit(): void { | ||
| this._configService.scheme$.subscribe((scheme) => { | ||
| this.scheme = scheme | ||
| }) | ||
| this._setOverlay() | ||
| this._updateSortState() | ||
| this.pinState = this.column.isPinned() | ||
| this.column.addEventListener('sortChanged', () => { | ||
| this._updateSortState() | ||
| }) | ||
| this.gridApi.addEventListener('columnPinned', () => { | ||
| this.pinState = this.column.isPinned() | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Fixed in the latest commit. Added OnDestroy implementation that calls _unsubscribeAll$.next()/.complete() and overlayRef?.dispose(). Both the scheme$ subscription and the backdropClick() subscription now use takeUntil(this._unsubscribeAll$) to clean up when the component is destroyed.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: kflemin <2205659+kflemin@users.noreply.github.com>
…spose overlay and unsubscribe Co-authored-by: kflemin <2205659+kflemin@users.noreply.github.com>
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
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.
No description provided.