Skip to content

Fix/branding rename pr 137 - #141

Open
kavix wants to merge 18 commits into
guidewire-oss:mainfrom
kavix:fix/branding-rename-pr-137
Open

Fix/branding rename pr 137#141
kavix wants to merge 18 commits into
guidewire-oss:mainfrom
kavix:fix/branding-rename-pr-137

Conversation

@kavix

@kavix kavix commented Aug 7, 2026

Copy link
Copy Markdown

Summary by cubic

Rebrand the product from Team360 to Team Health Check across the app and docs, and improve organization branding config with a safe default and tests. Also switch the frontend test environment to happy-dom.

  • Refactors

    • Renamed all user-facing strings: UI, emails, logs, docs, Makefiles, and Grafana dashboards.
    • Backend: organization config now reads company name from app settings; falls back to "My Company" when unset; added integration tests.
    • Frontend: updated default branding fallback and page metadata to "Team Health Check".
  • Dependencies

    • Frontend tests now run with happy-dom (replacing jsdom) via Vitest config.
    • Added "type": "module" to frontend/package.json.

Written for commit d4dccb9. Summary will update on new commits.

Review in cubic

kavix and others added 17 commits July 29, 2026 19:58
Replace Team360 branding with Team Health Check across the project
documentation, frontend metadata, backend documentation, Makefiles,
Grafana dashboard titles, and UI fallback branding to reflect the new
project identity.

Changes include:
- Update project documentation and architecture guides
- Rename backend/frontend README titles
- Update Makefile output messages
- Rename Grafana dashboard titles and descriptions
- Update frontend metadata and default application name
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Updated all instances of 'Team360' to 'Team Health Check' for consistency throughout the README.

Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Fix application name in start-dev.sh
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 issues found across 36 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/README.md">

<violation number="1" location="frontend/README.md:3">
P3: Line 3 now reads "the Team Health Check Squad Health Check system" — redundant since "Health Check" already appears in "Squad Health Check". Suggest cleaning up to "the Squad Health Check system" or "the Team Health Check system".</violation>
</file>

<file name="backend/infrastructure/persistence/postgres/organization_repository.go">

<violation number="1" location="backend/infrastructure/persistence/postgres/organization_repository.go:29">
P2: When branding settings need to be read, Get() now calls GetAppSettings() but silently discards any non-"no rows" database error (for example a connectivity or query failure) and falls back to the default company name while still returning a nil error. Since Get() already propagates the hierarchy-level query error to its callers, this path masks real database problems as if the config loaded successfully, which makes failures hard to diagnose. Consider treating a genuine GetAppSettings error like the hierarchy-level error and propagating it, reserving the silent default only for the intended no-row/empty-branding case.</violation>
</file>

<file name="frontend/package.json">

<violation number="1" location="frontend/package.json:5">
P0: Adding `"type": "module"` to package.json treats `postcss.config.js` and `tailwind.config.js` as ES modules, but both are authored in CommonJS (`module.exports = {...}`). Node will now fail to load them with `module is not defined in ES module scope`, which breaks `npm run dev` and the `npm run build` step in the CI `frontend-build` job. Since the PR purpose is a branding rename, this config change looks unintended. Convert both configs to ESM (`export default {...}`) or rename them to `postcss.config.cjs` / `tailwind.config.cjs` to keep them CommonJS, then drop the `"type": "module"` field if it's not needed for the test setup.</violation>

<violation number="2" location="frontend/package.json:49">
P3: After switching the vitest environment to happy-dom, `jsdom` is no longer referenced anywhere in the frontend (its only usage was the `environment: 'jsdom'` that this PR replaces). The `jsdom` devDependency is now dead weight in the lockfile and install. Consider removing it from package.json for cleanliness.</violation>
</file>

<file name="backend/infrastructure/email/templates.go">

<violation number="1" location="backend/infrastructure/email/templates.go:140">
P3: The re-branded email footer now reads "Team Health Check — Team Health Check Platform", which is redundant since the same template's header already says "Team Health Check". Consider shortening the footer to just "Team Health Check Platform" (or another distinct tagline) so the footer copy reads cleanly in the emails users receive.</violation>
</file>

<file name="backend/tests/integration/organization_repository_test.go">

<violation number="1" location="backend/tests/integration/organization_repository_test.go:34">
P3: The fallback tests only exercise the row-missing path (DELETE FROM app_settings), so the 'no company_name configured' branch named in the Context is never tested. Get() decides with `settings.CompanyName != ""`; if a row exists with an empty company_name (reachable after a clear/update), a regression in that branch would pass silently. Consider adding a case that inserts an app_settings row with an empty company_name and asserts Get() still falls back to 'My Company'.</violation>
</file>

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread frontend/package.json
"name": "team360-health-check",
"version": "0.1.0",
"private": true,
"type": "module",

@cubic-dev-ai cubic-dev-ai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: Adding "type": "module" to package.json treats postcss.config.js and tailwind.config.js as ES modules, but both are authored in CommonJS (module.exports = {...}). Node will now fail to load them with module is not defined in ES module scope, which breaks npm run dev and the npm run build step in the CI frontend-build job. Since the PR purpose is a branding rename, this config change looks unintended. Convert both configs to ESM (export default {...}) or rename them to postcss.config.cjs / tailwind.config.cjs to keep them CommonJS, then drop the "type": "module" field if it's not needed for the test setup.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/package.json, line 5:

<comment>Adding `"type": "module"` to package.json treats `postcss.config.js` and `tailwind.config.js` as ES modules, but both are authored in CommonJS (`module.exports = {...}`). Node will now fail to load them with `module is not defined in ES module scope`, which breaks `npm run dev` and the `npm run build` step in the CI `frontend-build` job. Since the PR purpose is a branding rename, this config change looks unintended. Convert both configs to ESM (`export default {...}`) or rename them to `postcss.config.cjs` / `tailwind.config.cjs` to keep them CommonJS, then drop the `"type": "module"` field if it's not needed for the test setup.</comment>

<file context>
@@ -2,6 +2,7 @@
   "name": "team360-health-check",
   "version": "0.1.0",
   "private": true,
+  "type": "module",
   "scripts": {
     "dev": "next dev",
</file context>
Fix with cubic

config.ID = "default"
config.CompanyName = "Team360"
config.CompanyName = "My Company"
if settings, err := r.GetAppSettings(ctx); err == nil && settings.CompanyName != "" {

@cubic-dev-ai cubic-dev-ai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When branding settings need to be read, Get() now calls GetAppSettings() but silently discards any non-"no rows" database error (for example a connectivity or query failure) and falls back to the default company name while still returning a nil error. Since Get() already propagates the hierarchy-level query error to its callers, this path masks real database problems as if the config loaded successfully, which makes failures hard to diagnose. Consider treating a genuine GetAppSettings error like the hierarchy-level error and propagating it, reserving the silent default only for the intended no-row/empty-branding case.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrastructure/persistence/postgres/organization_repository.go, line 29:

<comment>When branding settings need to be read, Get() now calls GetAppSettings() but silently discards any non-"no rows" database error (for example a connectivity or query failure) and falls back to the default company name while still returning a nil error. Since Get() already propagates the hierarchy-level query error to its callers, this path masks real database problems as if the config loaded successfully, which makes failures hard to diagnose. Consider treating a genuine GetAppSettings error like the hierarchy-level error and propagating it, reserving the silent default only for the intended no-row/empty-branding case.</comment>

<file context>
@@ -23,9 +23,12 @@ func NewOrganizationRepository(db *sql.DB) organization.Repository {
 	config.ID = "default"
-	config.CompanyName = "Team360"
+	config.CompanyName = "My Company"
+	if settings, err := r.GetAppSettings(ctx); err == nil && settings.CompanyName != "" {
+		config.CompanyName = settings.CompanyName
+	}
</file context>
Fix with cubic

</td></tr>
<tr><td style="background:#F9FAFB;padding:16px 32px;text-align:center;">
<p style="margin:0;color:#9CA3AF;font-size:11px;">Teams360 — Team Health Check Platform</p>
<p style="margin:0;color:#9CA3AF;font-size:11px;">Team Health Check — Team Health Check Platform</p>

@cubic-dev-ai cubic-dev-ai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The re-branded email footer now reads "Team Health Check — Team Health Check Platform", which is redundant since the same template's header already says "Team Health Check". Consider shortening the footer to just "Team Health Check Platform" (or another distinct tagline) so the footer copy reads cleanly in the emails users receive.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrastructure/email/templates.go, line 140:

<comment>The re-branded email footer now reads "Team Health Check — Team Health Check Platform", which is redundant since the same template's header already says "Team Health Check". Consider shortening the footer to just "Team Health Check Platform" (or another distinct tagline) so the footer copy reads cleanly in the emails users receive.</comment>

<file context>
@@ -137,7 +137,7 @@ func RenderIndividualSurveyEmail(data IndividualSurveyEmailData) string {
   </td></tr>
   <tr><td style="background:#F9FAFB;padding:16px 32px;text-align:center;">
-    <p style="margin:0;color:#9CA3AF;font-size:11px;">Teams360 — Team Health Check Platform</p>
+    <p style="margin:0;color:#9CA3AF;font-size:11px;">Team Health Check — Team Health Check Platform</p>
   </td></tr>
 </table>
</file context>
Fix with cubic

Comment thread frontend/README.md Outdated

Describe("Get CompanyName fallback and configuration loading", func() {
Context("when app_settings has no company_name configured or row is missing", func() {
It("falls back to 'My Company'", func() {

@cubic-dev-ai cubic-dev-ai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The fallback tests only exercise the row-missing path (DELETE FROM app_settings), so the 'no company_name configured' branch named in the Context is never tested. Get() decides with settings.CompanyName != ""; if a row exists with an empty company_name (reachable after a clear/update), a regression in that branch would pass silently. Consider adding a case that inserts an app_settings row with an empty company_name and asserts Get() still falls back to 'My Company'.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/tests/integration/organization_repository_test.go, line 34:

<comment>The fallback tests only exercise the row-missing path (DELETE FROM app_settings), so the 'no company_name configured' branch named in the Context is never tested. Get() decides with `settings.CompanyName != ""`; if a row exists with an empty company_name (reachable after a clear/update), a regression in that branch would pass silently. Consider adding a case that inserts an app_settings row with an empty company_name and asserts Get() still falls back to 'My Company'.</comment>

<file context>
@@ -0,0 +1,65 @@
+
+	Describe("Get CompanyName fallback and configuration loading", func() {
+		Context("when app_settings has no company_name configured or row is missing", func() {
+			It("falls back to 'My Company'", func() {
+				// Clear any app_settings row if present
+				_, err := db.ExecContext(ctx, "DELETE FROM app_settings")
</file context>
Fix with cubic

Comment thread frontend/package.json
"autoprefixer": "^10.4.21",
"eslint": "^8.57.1",
"eslint-config-next": "^15.5.12",
"happy-dom": "^20.11.1",

@cubic-dev-ai cubic-dev-ai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: After switching the vitest environment to happy-dom, jsdom is no longer referenced anywhere in the frontend (its only usage was the environment: 'jsdom' that this PR replaces). The jsdom devDependency is now dead weight in the lockfile and install. Consider removing it from package.json for cleanliness.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/package.json, line 49:

<comment>After switching the vitest environment to happy-dom, `jsdom` is no longer referenced anywhere in the frontend (its only usage was the `environment: 'jsdom'` that this PR replaces). The `jsdom` devDependency is now dead weight in the lockfile and install. Consider removing it from package.json for cleanliness.</comment>

<file context>
@@ -45,6 +46,7 @@
     "autoprefixer": "^10.4.21",
     "eslint": "^8.57.1",
     "eslint-config-next": "^15.5.12",
+    "happy-dom": "^20.11.1",
     "jsdom": "^27.2.0",
     "postcss": "^8.5.6",
</file context>
Fix with cubic

Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Signed-off-by: Kavindu Sachinthe <kavix@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants