Tenant Based Micro-Improvement Dashboard Creation#185
Open
prashanthShiksha wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThe PR introduces tenant-level scoping to Metabase admin dashboards. It threads a Changes
Sequence DiagramsequenceDiagram
participant PDFunc as ProjectMetabaseDashboardFunction
participant PAC as ProcessAdminConstructor
participant UAATF as UpdateAndAddAdminTenantFilter
participant MU as MetabaseUtil
participant DB as Metabase Database
PDFunc->>PAC: ProcessAndUpdateJsonFiles(projects, solutions, tenantId, ...)
PAC->>PAC: updatePostgresDatabaseQuery(tenantId)
PAC->>PAC: Replace ${tenant_id} with tenantId value in SQL
PDFunc->>UAATF: updateAndAddFilter(metabaseUtil, queryResult, tenantId, ...)
UAATF->>UAATF: Replace ${tenant_id} in JSON fields
UAATF->>UAATF: Update collection_id and database_id
UAATF->>MU: createQuestionCard(updatedCard)
MU->>DB: POST /api/card
DB-->>MU: {id: ..., created_at: ...}
MU-->>UAATF: Response
UAATF->>UAATF: Extract question id
UAATF-->>PDFunc: Return question id
PDFunc->>PDFunc: Build slug -> filter mapping
PDFunc->>MU: updateParameterFunction(mapping, dashboardId)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
metabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/big-number/4_no_of_unique_user_consuming_the_project.json (1)
174-248:⚠️ Potential issue | 🟠 MajorFix the stale dashboard card ID in
parameter_mappings.
dashCards.card_idis6939, but everyparameter_mappings[].card_idis8985. The parameter filters will bind to the wrong dashboard card. Update allparameter_mappings[].card_idvalues to6939to match the dashboard card ID.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@metabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/big-number/4_no_of_unique_user_consuming_the_project.json` around lines 174 - 248, The parameter_mappings entries are pointing to the wrong dashboard card (they all use card_id 8985) while the enclosing card's card_id is 6939; update every parameter_mappings[].card_id (for parameter_id values like "c32c8fc5", "74a10335", "8c7d86ea", "6e5c08dc", "efa886d3", "5088d3e5") to 6939 so the parameter filters bind to the correct dashboard card_id 6939.metabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/functions/ProcessAdminConstructor.scala (1)
125-136:⚠️ Potential issue | 🟠 MajorEscape
tenantIdbefore building the SQL literal.The new
${tenant_id}replacement does not escape quotes insidetenantId. If a tenant contains', the generated native query becomes invalid and the admin card creation path can fail.Proposed fix
+ val escapedTenantId = tenantId.replace("'", "''") val updatedQuery = queryNode.asText() .replace("${config.projects}", projectsTable) .replace("${config.solutions}", solutionsTable) - .replace("${tenant_id}", s"'$tenantId'") + .replace("${tenant_id}", s"'$escapedTenantId'")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@metabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/functions/ProcessAdminConstructor.scala` around lines 125 - 136, The tenantId is inserted into the SQL literal without escaping in updatePostgresDatabaseQuery, risking broken SQL if tenantId contains a single quote; before building updatedQuery (the string replaced via queryNode.asText().replace(...)), escape any single quotes in tenantId (e.g., replace "'" with "''") and then wrap it in the surrounding single quotes so the replacement uses the escaped value; update the replacement for "${tenant_id}" to use the escaped tenantId variable so query generation remains valid.metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProjectMetabaseDashboardFunction.scala (1)
107-112:⚠️ Potential issue | 🟠 MajorUse the tenant-scoped collection name in the existence check.
The guard still looks for
"Micro Improvements", but Line 298 creates"Micro Improvements [TenantId: $tenantId]". Reruns will miss the existing tenant collection and recreate dashboards/cards under it.Proposed fix
- val (mipCollectionPresent, mipCollectionId) = validateCollection("Micro Improvements", "Admin") + val mipCollectionName = s"Micro Improvements [TenantId: $tenantId]" + val (mipCollectionPresent, mipCollectionId) = validateCollection(mipCollectionName, "Admin")As per coding guidelines
metabase-jobs/**: "Verify: - Idempotency of ETL runs".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProjectMetabaseDashboardFunction.scala` around lines 107 - 112, The existence check currently calls validateCollection("Micro Improvements", "Admin") but the collection is created as a tenant-scoped name ("Micro Improvements [TenantId: $tenantId]"); update the guard in ProjectMetabaseDashboardFunction (around the code that calls validateCollection and createMicroImprovementsCollectionAndDashboard) to build the tenant-scoped collection name (e.g., val collectionName = s"Micro Improvements [TenantId: $tenantId]") and pass that collectionName into validateCollection instead of the hard-coded "Micro Improvements" so reruns correctly detect the existing tenant collection and skip recreation.metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProcessAdminConstructor.scala (1)
131-142:⚠️ Potential issue | 🟠 MajorEscape
tenantIdbefore injecting it into the SQL literal.The new replacement wraps
tenantIdin quotes but does not escape quotes inside the value. A tenant likefoo'barwill produce invalid SQL in the generated Metabase card and can alter the predicate unexpectedly.Proposed fix
+ val escapedTenantId = tenantId.replace("'", "''") val updatedQuery = queryNode.asText() .replace("${config.projects}", projectsTable) .replace("${config.solutions}", solutionsTable) - .replace("${tenant_id}", s"'$tenantId'") + .replace("${tenant_id}", s"'$escapedTenantId'")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProcessAdminConstructor.scala` around lines 131 - 142, In updatePostgresDatabaseQuery, the tenantId is injected into the SQL string without escaping, which breaks SQL if tenantId contains single quotes; before replacing "${tenant_id}" compute an escapedTenant by replacing any single quote characters with two single quotes (SQL-escape) and then use that escapedTenant when forming the quoted literal for the .replace call so the generated query uses a safe SQL string literal.
🧹 Nitpick comments (1)
metabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/graph/2_status_of_improvement_programs.json (1)
87-87: Validate composite indexing for tenant-filtered dashboard paths.Line 87 now filters on
tenant_idplus multiple dimension IDs; please confirm supporting indexes to keep dashboard queries fast under load.As per coding guidelines "Verify: - SQL query performance and index usage".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@metabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/graph/2_status_of_improvement_programs.json` at line 87, The query filters projects by tenant_id plus various dimension IDs and joins to solutions; ensure composite indexes exist on the projects table for (tenant_id, state_id), (tenant_id, district_id), (tenant_id, program_id), (tenant_id, block_id), (tenant_id, cluster_id), and (tenant_id, org_id), and an index supporting the join (solution_id) or composite (tenant_id, solution_id) if selective; if any are missing create them, then validate with EXPLAIN/ANALYZE on the query using the relevant symbols (${config.projects}, ${config.solutions}, tenant_id, state_id, district_id, program_id, block_id, cluster_id, org_id, solution_id) and update stats (ANALYZE) to ensure the planner uses the indexes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProjectMetabaseDashboardFunction.scala`:
- Around line 248-249: The programCollectionDescription currently hardcodes
"Collection For: Admin" which breaks idempotency because validateCollection(...,
"Program Manager", ...) expects "Program Manager"; update the
programCollectionDescription string (the variable programCollectionDescription
in ProjectMetabaseDashboardFunction.scala) to use "Collection For: Program
Manager" so the description matches the validateCollection lookup and reruns
will find the existing Program Manager collection.
- Around line 318-331: The loop stores a sentinel -1 from
UpdateAndAddAdminTenantFilter.updateAndAddFilter into
slugNameToTenantIdFilterMap which later gets persisted by
UpdateParameters.updateParameterFunction; change the loop in
ProjectMetabaseDashboardFunction so you only put entries into
slugNameToTenantIdFilterMap when tenantIdFilter != -1 (i.e., successful
creation), skip/omit failed entries, and optionally log a warning including
slugName and the failure; pass the resulting immutable map (containing only
valid positive card IDs) to UpdateParameters.updateParameterFunction.
In
`@metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/UpdateAndAddAdminTenantFilter.scala`:
- Around line 74-87: The function getTheQuestionId should not return -1 on
failure; change its signature to return Option[Int] (i.e., def
getTheQuestionId(json: JsonNode): Option[Int]) and remove printing/returning -1;
on success return Some(id) and on any parse/create failure return None (or
rethrow a descriptive exception if you prefer fail-fast). Update all callers
that assign into values_source_config.card_id to handle None (skip mapping or
fail-fast), and ensure metabaseUtil.createQuestionCard errors are caught and
propagated as None or an exception so invalid card ids are never written as -1.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/graph/1_submission_status_breakdown.json`:
- Line 87: The WHERE clause uses an unqualified tenant_id predicate; change the
predicate to qualify the column with the projects alias (replace "tenant_id =
${tenant_id}" with "${config.projects}.tenant_id = ${tenant_id}") inside the
query string, and apply the same qualification pattern to any other tenant_id
predicates in this Admin query (including those inside the subselect WHERE
clauses) to match the usage of ${config.projects} in JOINed queries like
select_program.json.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/Submission-Details-CSV/json/table/1_submission_data.json`:
- Line 87: Update the unqualified tenant filter so it is explicitly scoped to
the projects table: replace occurrences of "AND tenant_id = ${tenant_id}" with
"AND ${config.projects}.tenant_id = ${tenant_id}" in the query blocks (e.g.,
within the SELECT query that references ${config.projects} and
${config.solutions} in 1_submission_data.json) and apply the same change to the
other three Admin files that use ${config.projects}
(User-Details/json/graph/1_submission_status_breakdown.json,
Project-Details/json/table/1_submission_data.json,
Project-Details/json/graph/1_submission_status_breakdown.json) so all tenant
predicates are qualified as ${config.projects}.tenant_id.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/big-number/3_no_of_unique_user_submitted_the_project.json`:
- Line 9: The code currently substitutes ${tenant_id} into SQL via
updatePostgresDatabaseQuery without escaping, allowing quotes to break the
query; modify updatePostgresDatabaseQuery so it escapes single quotes in
tenantId before doing the .replace (i.e., replace each ' with '' in tenantId)
or, better, refactor to use a parameterized query instead of raw string
substitution; ensure you update the substitution call that targets
"${tenant_id}" and validate with a tenantId containing a single quote to confirm
safety.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/big-number/4_no_of_unique_user_consuming_the_project.json`:
- Line 9: The JSON "query" contains an unresolved placeholder ${tenant_id} which
will be stored verbatim by the config-data-loader; either replace ${tenant_id}
in the "query" string with a concrete tenant ID value before committing, or add
substitution logic to the config-data-loader (similar to dashboard-creator) so
that when loading the "query" field it replaces ${tenant_id} with the runtime
tenant value; update the loader to detect the ${tenant_id} token in the "query"
key and perform the replacement prior to inserting the JSON into the database.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/graph/2_status_of_user_program_wise.json`:
- Line 87: The query labels COUNT(DISTINCT ${config.projects}.created_by) as
"Total Projects" which is incorrect since it counts distinct users; rename that
alias to "Total Users" and update the ORDER BY clause from ORDER BY "Total
Projects" DESC to ORDER BY "Total Users" DESC so the label and sort match the
${config.projects}.created_by aggregation.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/graph/3_status_of_user_state_wise.json`:
- Line 87: The WHERE clause uses an unqualified tenant_id causing ambiguity
after the join; change the predicate to explicitly reference the projects table
by replacing tenant_id = ${tenant_id} with ${config.projects}.tenant_id =
${tenant_id} (update predicates inside the main WHERE and any nested SELECTs
that use tenant_id in the subqueries), ensuring all tenant filters consistently
reference ${config.projects}.tenant_id and leaving other table references
unchanged.
In
`@metabase-jobs/config-data-loader/projectJson/Project/Project-Filters/json/admin-filter/select_organisation.json`:
- Line 9: The query in the "query" property currently hardcodes 'TENANTID' which
is inconsistent with other pipeline queries; update the SQL string used in the
query field (the SELECT distinct(org_name) FROM ${config.projects} ... entry) to
replace the literal 'TENANTID' with the ${tenant_id} placeholder so the runtime
substitution matches the rest of the pipeline and avoids reliance on
UpdateAndAddAdminTenantFilter to mutate a literal.
In
`@metabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/functions/ProjectMetabaseDashboardFunction.scala`:
- Around line 315-328: The loop stores a -1 return from
UpdateAndAddAdminTenantFilter.updateAndAddFilter on failure, which later causes
bad card_id values; change the loop to fail-fast: after calling
UpdateAndAddAdminTenantFilter.updateAndAddFilter (referenced symbol), check if
tenantIdFilter == -1 and if so throw a RuntimeException (or otherwise abort)
with a clear message including slugName and configString; only insert into
slugNameToTenantIdFilterMap when tenantIdFilter != -1 so
UpdateParameters.updateParameterFunction receives only valid card IDs.
In
`@metabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/functions/UpdateAndAddAdminTenantFilter.scala`:
- Around line 74-87: The getTheQuestionId function currently returns -1 on
failures which is interpreted as a valid card_id downstream; change
getTheQuestionId to fail-fast or return an explicit failure type (e.g.,
Option[Int] or Either[Throwable, Int]) instead of -1 so callers can skip or
handle missing cards; update the method signature (getTheQuestionId) and replace
the printlns with proper error propagation (throw the caught exception or return
None/Left with the error), and ensure callers handle the new return type to
avoid poisoning the tenant filter map.
---
Outside diff comments:
In
`@metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProcessAdminConstructor.scala`:
- Around line 131-142: In updatePostgresDatabaseQuery, the tenantId is injected
into the SQL string without escaping, which breaks SQL if tenantId contains
single quotes; before replacing "${tenant_id}" compute an escapedTenant by
replacing any single quote characters with two single quotes (SQL-escape) and
then use that escapedTenant when forming the quoted literal for the .replace
call so the generated query uses a safe SQL string literal.
In
`@metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProjectMetabaseDashboardFunction.scala`:
- Around line 107-112: The existence check currently calls
validateCollection("Micro Improvements", "Admin") but the collection is created
as a tenant-scoped name ("Micro Improvements [TenantId: $tenantId]"); update the
guard in ProjectMetabaseDashboardFunction (around the code that calls
validateCollection and createMicroImprovementsCollectionAndDashboard) to build
the tenant-scoped collection name (e.g., val collectionName = s"Micro
Improvements [TenantId: $tenantId]") and pass that collectionName into
validateCollection instead of the hard-coded "Micro Improvements" so reruns
correctly detect the existing tenant collection and skip recreation.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/big-number/4_no_of_unique_user_consuming_the_project.json`:
- Around line 174-248: The parameter_mappings entries are pointing to the wrong
dashboard card (they all use card_id 8985) while the enclosing card's card_id is
6939; update every parameter_mappings[].card_id (for parameter_id values like
"c32c8fc5", "74a10335", "8c7d86ea", "6e5c08dc", "efa886d3", "5088d3e5") to 6939
so the parameter filters bind to the correct dashboard card_id 6939.
In
`@metabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/functions/ProcessAdminConstructor.scala`:
- Around line 125-136: The tenantId is inserted into the SQL literal without
escaping in updatePostgresDatabaseQuery, risking broken SQL if tenantId contains
a single quote; before building updatedQuery (the string replaced via
queryNode.asText().replace(...)), escape any single quotes in tenantId (e.g.,
replace "'" with "''") and then wrap it in the surrounding single quotes so the
replacement uses the escaped value; update the replacement for "${tenant_id}" to
use the escaped tenantId variable so query generation remains valid.
---
Nitpick comments:
In
`@metabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/graph/2_status_of_improvement_programs.json`:
- Line 87: The query filters projects by tenant_id plus various dimension IDs
and joins to solutions; ensure composite indexes exist on the projects table for
(tenant_id, state_id), (tenant_id, district_id), (tenant_id, program_id),
(tenant_id, block_id), (tenant_id, cluster_id), and (tenant_id, org_id), and an
index supporting the join (solution_id) or composite (tenant_id, solution_id) if
selective; if any are missing create them, then validate with EXPLAIN/ANALYZE on
the query using the relevant symbols (${config.projects}, ${config.solutions},
tenant_id, state_id, district_id, program_id, block_id, cluster_id, org_id,
solution_id) and update stats (ANALYZE) to ensure the planner uses the indexes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0d40f18c-7f4f-4c58-b158-de8c4033b748
📒 Files selected for processing (35)
metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProcessAdminConstructor.scalametabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProjectMetabaseDashboardFunction.scalametabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/UpdateAndAddAdminTenantFilter.scalametabase-jobs/combined-dashboard-creator/src/test/scala/org/shikshalokam/job/combined/dashboard/creator/spec/ProjectMetabaseDashboardFunctionTestSpec.scalametabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/big-number/1_no_of_project_started.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/big-number/2_no_of_project_inprogress.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/big-number/3_no_of_project_submitted.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/big-number/4_no_of_unique_assigned_project_on_platform.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/big-number/5_no_of_project_submitted_with_evidence.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/big-number/6_no_of_certificates_issued.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/graph/1_submission_status_breakdown.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/graph/2_status_of_improvement_programs.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/graph/3_status_of_improvement_state_wise.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/graph/4_status_of_improvement_district_wise.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/table/1_submission_data.jsonmetabase-jobs/config-data-loader/projectJson/Admin/Submission-Details-CSV/json/table/1_submission_data.jsonmetabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/big-number/1_no_of_unique_user_started_the_project.jsonmetabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/big-number/2_no_of_unique_user_in_progress.jsonmetabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/big-number/3_no_of_unique_user_submitted_the_project.jsonmetabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/big-number/4_no_of_unique_user_consuming_the_project.jsonmetabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/big-number/5_no_of_unique_user_submitted_with_evidence_till_date.jsonmetabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/graph/1_submission_status_breakdown.jsonmetabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/graph/2_status_of_user_program_wise.jsonmetabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/graph/3_status_of_user_state_wise.jsonmetabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/graph/4_status_of_user_district_wise.jsonmetabase-jobs/config-data-loader/projectJson/Project/Project-Filters/json/admin-filter/select_block.jsonmetabase-jobs/config-data-loader/projectJson/Project/Project-Filters/json/admin-filter/select_cluster.jsonmetabase-jobs/config-data-loader/projectJson/Project/Project-Filters/json/admin-filter/select_district.jsonmetabase-jobs/config-data-loader/projectJson/Project/Project-Filters/json/admin-filter/select_organisation.jsonmetabase-jobs/config-data-loader/projectJson/Project/Project-Filters/json/admin-filter/select_program.jsonmetabase-jobs/config-data-loader/projectJson/Project/Project-Filters/json/admin-filter/select_state.jsonmetabase-jobs/config-data-loader/projectJson/Project/Project-Parameter/json/admin-parameter/admin-parameter.jsonmetabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/functions/ProcessAdminConstructor.scalametabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/functions/ProjectMetabaseDashboardFunction.scalametabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/functions/UpdateAndAddAdminTenantFilter.scala
Comment on lines
+248
to
+249
| val programCollectionName = s"$programName" | ||
| val programCollectionDescription = s"Program Id: $targetedProgramId\n\nProgram External Id: $programExternalId\n\nCreator Organisation: $orgId\n\nCollection For: Admin\n\nProgram Description: $programDescription" |
There was a problem hiding this comment.
Keep the Program Manager collection tagged as Program Manager.
This branch creates Program Manager collections, but the description now says Collection For: Admin. The later validateCollection(..., "Program Manager", ...) lookup uses that description, so reruns will fail to match the existing collection.
Proposed fix
- val programCollectionDescription = s"Program Id: $targetedProgramId\n\nProgram External Id: $programExternalId\n\nCreator Organisation: $orgId\n\nCollection For: Admin\n\nProgram Description: $programDescription"
+ val programCollectionDescription = s"Program Id: $targetedProgramId\n\nProgram External Id: $programExternalId\n\nCreator Organisation: $orgId\n\nCollection For: Program Manager\n\nProgram Description: $programDescription"As per coding guidelines metabase-jobs/**: "Verify: - Idempotency of ETL runs".
📝 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.
Suggested change
| val programCollectionName = s"$programName" | |
| val programCollectionDescription = s"Program Id: $targetedProgramId\n\nProgram External Id: $programExternalId\n\nCreator Organisation: $orgId\n\nCollection For: Admin\n\nProgram Description: $programDescription" | |
| val programCollectionName = s"$programName" | |
| val programCollectionDescription = s"Program Id: $targetedProgramId\n\nProgram External Id: $programExternalId\n\nCreator Organisation: $orgId\n\nCollection For: Program Manager\n\nProgram Description: $programDescription" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProjectMetabaseDashboardFunction.scala`
around lines 248 - 249, The programCollectionDescription currently hardcodes
"Collection For: Admin" which breaks idempotency because validateCollection(...,
"Program Manager", ...) expects "Program Manager"; update the
programCollectionDescription string (the variable programCollectionDescription
in ProjectMetabaseDashboardFunction.scala) to use "Collection For: Program
Manager" so the description matches the validateCollection lookup and reruns
will find the existing Program Manager collection.
Comment on lines
+318
to
+331
| val filterQuery: String = s"SELECT config FROM $reportConfig WHERE report_name = 'Project-Filters' AND question_type = 'admin-filter'" | ||
| val filterResults: List[Map[String, Any]] = postgresUtil.fetchData(filterQuery) | ||
| val objectMapper = new ObjectMapper() | ||
| val slugNameToTenantIdFilterMap = mutable.Map[String, Int]() | ||
| for (result <- filterResults) { | ||
| val configString = result.get("config").map(_.toString).getOrElse("") | ||
| val configJson = objectMapper.readTree(configString) | ||
| val slugName = configJson.findValue("name").asText() | ||
| val tenantIdFilter: Int = UpdateAndAddAdminTenantFilter.updateAndAddFilter(metabaseUtil, configJson: JsonNode, s"$tenantId", mipCollectionId, databaseId, projects, solutions) | ||
| slugNameToTenantIdFilterMap(slugName) = tenantIdFilter | ||
| } | ||
| val immutableSlugNameToTenantIdFilterMap: Map[String, Int] = slugNameToTenantIdFilterMap.toMap | ||
| val parametersQuery: String = s"SELECT config FROM $reportConfig WHERE report_name = 'Project-Parameter' AND question_type = 'admin-parameter'" | ||
| UpdateParameters.updateParameterFunction(metabaseUtil, postgresUtil, parametersQuery, immutableSlugNameToTenantIdFilterMap, mipDashboardId) |
There was a problem hiding this comment.
Do not persist invalid filter card IDs into admin parameters.
UpdateAndAddAdminTenantFilter.updateAndAddFilter can return -1 on failure, and this loop currently stores it anyway. That bad ID is then written into values_source_config.card_id, which breaks the admin filter wiring.
Proposed fix
val slugName = configJson.findValue("name").asText()
val tenantIdFilter: Int = UpdateAndAddAdminTenantFilter.updateAndAddFilter(metabaseUtil, configJson: JsonNode, s"$tenantId", mipCollectionId, databaseId, projects, solutions)
+ if (tenantIdFilter == -1) {
+ throw new IllegalStateException(s"Failed to create admin filter card for '$slugName'")
+ }
slugNameToTenantIdFilterMap(slugName) = tenantIdFilterAs per coding guidelines metabase-jobs/**: "Verify: - Metabase cards/filters mapping consistency".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/ProjectMetabaseDashboardFunction.scala`
around lines 318 - 331, The loop stores a sentinel -1 from
UpdateAndAddAdminTenantFilter.updateAndAddFilter into
slugNameToTenantIdFilterMap which later gets persisted by
UpdateParameters.updateParameterFunction; change the loop in
ProjectMetabaseDashboardFunction so you only put entries into
slugNameToTenantIdFilterMap when tenantIdFilter != -1 (i.e., successful
creation), skip/omit failed entries, and optionally log a warning including
slugName and the failure; pass the resulting immutable map (containing only
valid positive card IDs) to UpdateParameters.updateParameterFunction.
Comment on lines
+74
to
+87
| def getTheQuestionId(json: JsonNode): Int = { | ||
| try { | ||
| val requestBody = json.get("questionCard") | ||
| val questionCardResponse = metabaseUtil.createQuestionCard(requestBody.toString) | ||
| val responseJson = objectMapper.readTree(questionCardResponse) | ||
| Option(responseJson.get("id")).map(_.asInt()).getOrElse { | ||
| println("Error: 'id' field not found in the response.") | ||
| -1 | ||
| } | ||
| } catch { | ||
| case ex: Exception => | ||
| println(s"Error fetching 'id' from response: ${ex.getMessage}") | ||
| -1 | ||
| } |
There was a problem hiding this comment.
Do not return -1 for card creation failures.
-1 is treated as a real card_id downstream, so any Metabase create/parse failure will silently poison values_source_config.card_id and break the admin dashboard wiring. Fail fast here, or return an Option[Int]/Either and let the caller skip invalid mappings. As per coding guidelines: Metabase cards/filters mapping consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@metabase-jobs/combined-dashboard-creator/src/main/scala/org/shikshalokam/job/combined/dashboard/creator/functions/project/UpdateAndAddAdminTenantFilter.scala`
around lines 74 - 87, The function getTheQuestionId should not return -1 on
failure; change its signature to return Option[Int] (i.e., def
getTheQuestionId(json: JsonNode): Option[Int]) and remove printing/returning -1;
on success return Some(id) and on any parse/create failure return None (or
rethrow a descriptive exception if you prefer fail-fast). Update all callers
that assign into values_source_config.card_id to handle None (skip mapping or
fail-fast), and ensure metabaseUtil.createQuestionCard errors are caught and
propagated as None or an exception so invalid card ids are never written as -1.
| } | ||
| }, | ||
| "query": "SELECT \n CASE \n WHEN ${config.projects}.status = 'started' THEN 'Started'\n WHEN ${config.projects}.status = 'inProgress' THEN 'In Progress'\n WHEN ${config.projects}.status = 'submitted' THEN 'Submitted'\n ELSE ${config.projects}.status\n END AS \"Project Status\",\n COUNT(DISTINCT ${config.projects}.project_id) AS \"Total Projects\"\nFROM ${config.projects} \nJOIN ${config.solutions} ON ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\n AND ${config.solutions}.program_name IS NOT NULL\n [[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n LIMIT 1)\n ]]\n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n LIMIT 1\n )\n ]]\nGROUP BY ${config.projects}.status\nORDER BY \"Total Projects\" DESC;\n\n" | ||
| "query": "SELECT \n CASE \n WHEN ${config.projects}.status = 'started' THEN 'Started'\n WHEN ${config.projects}.status = 'inProgress' THEN 'In Progress'\n WHEN ${config.projects}.status = 'submitted' THEN 'Submitted'\n ELSE ${config.projects}.status\n END AS \"Project Status\",\n COUNT(DISTINCT ${config.projects}.project_id) AS \"Total Projects\"\nFROM ${config.projects} \nJOIN ${config.solutions} ON ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND tenant_id = ${tenant_id}\nAND ${config.solutions}.program_name IS NOT NULL\n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}}\n AND tenant_id = ${tenant_id} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\nGROUP BY ${config.projects}.status\nORDER BY \"Total Projects\" DESC;" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden -S '\$\{config\.solutions\}\.tenant_id|\$\{config\.projects\}\.tenant_id|JOIN \$\{config\.solutions\}' metabase-jobs/config-data-loader/projectJson/AdminRepository: ELEVATE-Project/data-pipeline
Length of output: 9802
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for schema definitions or config files that define these tables
find metabase-jobs/config-data-loader -type f \( -name "*.json" -o -name "*.sql" \) | head -20
# Search for any references to config.solutions and its columns
rg -i '"tenant_id"' metabase-jobs/config-data-loader/projectJson/Admin --type json -A 2 -B 2 | head -30
# Check if there's a database schema or config file
fd -e sql -e json metabase-jobs/config-data-loader | grep -i schema | head -10Repository: ELEVATE-Project/data-pipeline
Length of output: 2432
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for SQL files that might define schema
find metabase-jobs -type f -name "*.sql" 2>/dev/null | head -20
# Look for any explicit reference to config.solutions.tenant_id
rg 'config\.solutions.*tenant_id|config\.projects.*tenant_id' metabase-jobs/ --type json 2>/dev/null | head -5
# Check if there are any queries that explicitly qualify tenant_id with solutions
rg 'solutions.*\.tenant_id|solutions.*tenant' metabase-jobs/config-data-loader --type json 2>/dev/null | head -10Repository: ELEVATE-Project/data-pipeline
Length of output: 21477
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if there's a configuration that maps which columns belong to which tables
find metabase-jobs -type f -name "*config*" -o -name "*mapping*" | head -20
# Search for any reference to solutions having a tenant_id or looking for patterns where solutions columns are listed
rg 'solutions.*tenant|tenant.*solutions' metabase-jobs/config-data-loader --type json | head -10
# Look for any ETL or loader scripts that might define the schema
find metabase-jobs -type f \( -name "*.py" -o -name "*.js" -o -name "*.sh" \) | head -20Repository: ELEVATE-Project/data-pipeline
Length of output: 21320
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the data loader script
cat metabase-jobs/config-data-loader/data-loader.sh | head -100
# Look for any Python files that might define schema
find metabase-jobs -type f -name "*.py" | xargs grep -l "tenant" 2>/dev/null | head -10Repository: ELEVATE-Project/data-pipeline
Length of output: 2988
Qualify the tenant_id predicate for consistency with other JOIN queries.
The same JOIN pattern is used in select_program.json (Project-Filters) with explicit qualification: ${config.projects}.tenant_id = 'TENANTID'. For consistency, use ${config.projects}.tenant_id = ${tenant_id} instead of the unqualified tenant_id = ${tenant_id} here and in similar Admin queries. This follows the established pattern in the codebase and ensures clarity about which table the predicate applies to.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/Project-Details/json/graph/1_submission_status_breakdown.json`
at line 87, The WHERE clause uses an unqualified tenant_id predicate; change the
predicate to qualify the column with the projects alias (replace "tenant_id =
${tenant_id}" with "${config.projects}.tenant_id = ${tenant_id}") inside the
query string, and apply the same qualification pattern to any other tenant_id
predicates in this Admin query (including those inside the subselect WHERE
clauses) to match the usage of ${config.projects} in JOINed queries like
select_program.json.
| } | ||
| }, | ||
| "query": "SELECT \n ${config.solutions}.program_name AS \"Program\",\n ${config.solutions}.name AS \"Project\",\n ${config.projects}.state_name AS \"State\",\n ${config.projects}.district_name AS \"District\",\n${config.projects}.block_name AS \"Block\", \n${config.projects}.cluster_name AS \"Cluster\",\n ${config.projects}.org_name AS \"Organisation\",\n SUM(CASE WHEN ${config.projects}.status = 'started' THEN 1 ELSE 0 END) AS \"Started\",\n SUM(CASE WHEN ${config.projects}.status = 'inProgress' THEN 1 ELSE 0 END) AS \"In-Progress\",\n SUM(CASE WHEN ${config.projects}.status = 'inProgress' AND ${config.projects}.evidence IS NOT NULL AND TRIM(${config.projects}.evidence) <> '' THEN 1 ELSE 0 END) AS \"In-Progress projects with evidence\",\n SUM(CASE WHEN ${config.projects}.status = 'submitted' THEN 1 ELSE 0 END) AS \"Submitted\",\n SUM(CASE WHEN ${config.projects}.status = 'submitted' AND ${config.projects}.evidence IS NOT NULL AND TRIM(${config.projects}.evidence) <> '' THEN 1 ELSE 0 END) AS \"Submitted projects with evidences\"\nFROM ${config.projects} \nJOIN ${config.solutions} ON ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND ${config.projects}.state_name is not null \n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n LIMIT 1\n )\n ]]\nGROUP BY ${config.solutions}.program_name, ${config.solutions}.name,${config.projects}.state_name, ${config.projects}.district_name, \n${config.projects}.org_name,\n${config.projects}.block_name,\n${config.projects}.cluster_name\nORDER BY \"Submitted\" DESC;" | ||
| "query": "SELECT \n ${config.solutions}.program_name AS \"Program\",\n ${config.solutions}.name AS \"Project\",\n ${config.projects}.state_name AS \"State\",\n ${config.projects}.district_name AS \"District\",\n${config.projects}.block_name AS \"Block\", \n${config.projects}.cluster_name AS \"Cluster\",\n ${config.projects}.org_name AS \"Organisation\",\n SUM(CASE WHEN ${config.projects}.status = 'started' THEN 1 ELSE 0 END) AS \"Started\",\n SUM(CASE WHEN ${config.projects}.status = 'inProgress' THEN 1 ELSE 0 END) AS \"In-Progress\",\n SUM(CASE WHEN ${config.projects}.status = 'inProgress' AND ${config.projects}.evidence IS NOT NULL AND TRIM(${config.projects}.evidence) <> '' THEN 1 ELSE 0 END) AS \"In-Progress projects with evidence\",\n SUM(CASE WHEN ${config.projects}.status = 'submitted' THEN 1 ELSE 0 END) AS \"Submitted\",\n SUM(CASE WHEN ${config.projects}.status = 'submitted' AND ${config.projects}.evidence IS NOT NULL AND TRIM(${config.projects}.evidence) <> '' THEN 1 ELSE 0 END) AS \"Submitted projects with evidences\"\nFROM ${config.projects} \nJOIN ${config.solutions} ON ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND tenant_id = ${tenant_id}\nAND ${config.projects}.state_name is not null \n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}}\n AND tenant_id = ${tenant_id} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\nGROUP BY ${config.solutions}.program_name, ${config.solutions}.name,${config.projects}.state_name, ${config.projects}.district_name, \n${config.projects}.org_name,\n${config.projects}.block_name,\n${config.projects}.cluster_name\nORDER BY \"Submitted\" DESC;" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden -S '\$\{config\.solutions\}\.tenant_id|\$\{config\.projects\}\.tenant_id|JOIN \$\{config\.solutions\}' metabase-jobs/config-data-loader/projectJson/AdminRepository: ELEVATE-Project/data-pipeline
Length of output: 9802
🏁 Script executed:
#!/bin/bash
# Look for schema definitions, table creation scripts, or configuration files
fd -type f \( -name "*.sql" -o -name "*schema*" -o -name "*migration*" -o -name "*.ddl" \) 2>/dev/null | head -20Repository: ELEVATE-Project/data-pipeline
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Search for other JOIN queries involving solutions table to see if there's a broader pattern
rg -l 'JOIN.*\$\{config\.solutions\}' metabase-jobs/ | head -10Repository: ELEVATE-Project/data-pipeline
Length of output: 1287
🏁 Script executed:
#!/bin/bash
# Check if there are any config or initialization files that define table structures
fd -type f \( -name "*.json" -o -name "*.yaml" -o -name "*.yml" \) metabase-jobs/config-data-loader/ | xargs rg -l 'solutions.*tenant|tenant.*solutions' 2>/dev/null | head -5Repository: ELEVATE-Project/data-pipeline
Length of output: 867
🏁 Script executed:
#!/bin/bash
# Look at a different JOIN query file to see the pattern
head -100 metabase-jobs/config-data-loader/projectJson/District/Project-Details/json/table/project_status.json | cat -nRepository: ELEVATE-Project/data-pipeline
Length of output: 6370
🏁 Script executed:
#!/bin/bash
# Search for any database initialization or schema documentation
find metabase-jobs -type f -name "*.md" -o -name "README*" | xargs grep -l "tenant_id\|solutions\|projects" 2>/dev/null | head -5Repository: ELEVATE-Project/data-pipeline
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Check if there's a config loader or database setup script
find metabase-jobs -type f \( -name "*.scala" -o -name "*.java" -o -name "*.js" \) | xargs rg -l "tenant_id|solutions.*table|projects.*table" 2>/dev/null | head -5Repository: ELEVATE-Project/data-pipeline
Length of output: 783
🏁 Script executed:
#!/bin/bash
# Look at the Scala config files to understand the schema
head -150 metabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/task/ProjectMetabaseDashboardConfig.scala | cat -nRepository: ELEVATE-Project/data-pipeline
Length of output: 3001
🏁 Script executed:
#!/bin/bash
# Check if solutions table is mentioned with tenant in any config files
rg -A3 -B3 'solutions.*tenant|config\.solutions' metabase-jobs/project-dashboard-creator/src/main/scala/ | head -40Repository: ELEVATE-Project/data-pipeline
Length of output: 6257
🏁 Script executed:
#!/bin/bash
# Look at the specific query file under review more closely for any clues about the data model
grep -o '\${config\.[a-z]*}' metabase-jobs/config-data-loader/projectJson/Admin/Submission-Details-CSV/json/table/1_submission_data.json | sort | uniq -cRepository: ELEVATE-Project/data-pipeline
Length of output: 128
🏁 Script executed:
#!/bin/bash
# Check if the District version has tenant filtering and compare pattern consistency
grep -n 'tenant_id' metabase-jobs/config-data-loader/projectJson/District/Project-Details/json/table/project_status.json | head -5Repository: ELEVATE-Project/data-pipeline
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Search for any database schema or migration files that might define table structure
find metabase-jobs -name "*.sql" -o -name "*migration*" -o -name "*schema*" 2>/dev/null | xargs cat 2>/dev/null | grep -i "tenant_id\|create table\|solutions\|projects" | head -20Repository: ELEVATE-Project/data-pipeline
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Look for any documentation or README that explains the data model
find metabase-jobs -name "README*" -o -name "*.md" 2>/dev/null | xargs cat 2>/dev/null | grep -A5 -B5 "tenant\|solutions\|projects" | head -30Repository: ELEVATE-Project/data-pipeline
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Compare Admin vs District versions to understand the difference in tenant filtering
echo "=== Admin version with tenant_id filtering ==="
grep -c 'tenant_id' metabase-jobs/config-data-loader/projectJson/Admin/Submission-Details-CSV/json/table/1_submission_data.json
echo "=== District version (should have none or different pattern) ==="
grep -c 'tenant_id' metabase-jobs/config-data-loader/projectJson/District/Project-Details/json/table/project_status.json
echo "=== Check other Admin files for tenant pattern ==="
find metabase-jobs/config-data-loader/projectJson/Admin -name "*.json" | xargs grep -l 'tenant_id' | wc -l
echo "=== Total Admin json files ==="
find metabase-jobs/config-data-loader/projectJson/Admin -name "*.json" | wc -lRepository: ELEVATE-Project/data-pipeline
Length of output: 277
🏁 Script executed:
#!/bin/bash
# Check for any explicit column qualification patterns in other queries
rg '\${config\.[a-z_]+}\.[a-z_]+' metabase-jobs/config-data-loader/projectJson/Admin/ | head -20Repository: ELEVATE-Project/data-pipeline
Length of output: 187
🏁 Script executed:
#!/bin/bash
# Look at one more Admin query file to see if the unqualified tenant_id pattern is universal
head -150 metabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/graph/1_submission_status_breakdown.json | tail -80Repository: ELEVATE-Project/data-pipeline
Length of output: 3855
🏁 Script executed:
#!/bin/bash
# Look for any explicit field qualification to confirm the pattern
rg 'config\.projects\}\.tenant_id|config\.solutions\}\.tenant_id' metabase-jobs/Repository: ELEVATE-Project/data-pipeline
Length of output: 421
🏁 Script executed:
#!/bin/bash
# Check if the unqualified tenant_id pattern is used in ALL Admin queries consistently
find metabase-jobs/config-data-loader/projectJson/Admin -name "*.json" -exec grep -l 'tenant_id' {} \; | wc -l
find metabase-jobs/config-data-loader/projectJson/Admin -name "*.json" -exec grep -l 'JOIN.*solutions' {} \; | wc -lRepository: ELEVATE-Project/data-pipeline
Length of output: 78
🏁 Script executed:
#!/bin/bash
# Verify that all Admin queries with JOIN also have unqualified tenant_id
echo "=== Admin files with JOIN and tenant_id filtering ==="
find metabase-jobs/config-data-loader/projectJson/Admin -name "*.json" | while read f; do
if grep -q 'JOIN.*solutions' "$f" && grep -q 'tenant_id' "$f"; then
echo "✓ $f"
fi
done | wc -l
echo "=== Total Admin files with JOIN ==="
find metabase-jobs/config-data-loader/projectJson/Admin -name "*.json" | while read f; do
if grep -q 'JOIN.*solutions' "$f"; then
echo "$f"
fi
done | wc -lRepository: ELEVATE-Project/data-pipeline
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Check if District queries (which don't have tenant_id filtering) reveal why
head -200 metabase-jobs/config-data-loader/projectJson/District/Project-Details/json/table/project_status.json | grep -A2 "FROM\|JOIN\|WHERE"Repository: ELEVATE-Project/data-pipeline
Length of output: 2919
🏁 Script executed:
#!/bin/bash
# Examine the Project-Filters file more closely which shows explicit qualification
cat metabase-jobs/config-data-loader/projectJson/Project/Project-Filters/json/admin-filter/select_program.json | grep -o 'WHERE.*' | head -3Repository: ELEVATE-Project/data-pipeline
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Count how many times the pattern "config.projects}.tenant_id" appears (explicit qualification)
rg 'config\.projects.*tenant_id' metabase-jobs/config-data-loader/ | wc -lRepository: ELEVATE-Project/data-pipeline
Length of output: 76
🏁 Script executed:
#!/bin/bash
# Final verification: check if solutions table has tenant_id by looking for it in query output or schema hints
rg 'solutions.*tenant|program_name.*tenant' metabase-jobs/ | head -5Repository: ELEVATE-Project/data-pipeline
Length of output: 2103
Qualify the tenant_id predicate in all Admin JOIN queries.
The unqualified AND tenant_id = ${tenant_id} predicate in these queries deviates from the established pattern in the codebase, where explicit qualification (e.g., ${config.projects}.tenant_id) is standard. This affects 4 Admin files:
Submission-Details-CSV/json/table/1_submission_data.jsonUser-Details/json/graph/1_submission_status_breakdown.jsonProject-Details/json/table/1_submission_data.jsonProject-Details/json/graph/1_submission_status_breakdown.json
All 4 should be updated to explicitly qualify the tenant filter as ${config.projects}.tenant_id = ${tenant_id} for consistency and clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/Submission-Details-CSV/json/table/1_submission_data.json`
at line 87, Update the unqualified tenant filter so it is explicitly scoped to
the projects table: replace occurrences of "AND tenant_id = ${tenant_id}" with
"AND ${config.projects}.tenant_id = ${tenant_id}" in the query blocks (e.g.,
within the SELECT query that references ${config.projects} and
${config.solutions} in 1_submission_data.json) and apply the same change to the
other three Admin files that use ${config.projects}
(User-Details/json/graph/1_submission_status_breakdown.json,
Project-Details/json/table/1_submission_data.json,
Project-Details/json/graph/1_submission_status_breakdown.json) so all tenant
predicates are qualified as ${config.projects}.tenant_id.
| } | ||
| }, | ||
| "query": "SELECT DISTINCT(${config.solutions}.program_name) AS \"Programs\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'started' THEN ${config.projects}.created_by END) AS \"Started Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'inProgress' THEN ${config.projects}.created_by END) AS \"In Progress Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'submitted' THEN ${config.projects}.created_by END) AS \"Submitted Users\", \nCOUNT(DISTINCT ${config.projects}.created_by) AS \"Total Projects\" \nFROM ${config.projects} \njoin ${config.solutions} on ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND ${config.solutions}.program_name is not null\n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n LIMIT 1\n )\n ]]\nGROUP BY ${config.solutions}.program_name\nORDER BY \"Total Projects\" DESC;" | ||
| "query": "SELECT DISTINCT(${config.solutions}.program_name) AS \"Programs\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'started' THEN ${config.projects}.created_by END) AS \"Started Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'inProgress' THEN ${config.projects}.created_by END) AS \"In Progress Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'submitted' THEN ${config.projects}.created_by END) AS \"Submitted Users\", \nCOUNT(DISTINCT ${config.projects}.created_by) AS \"Total Projects\" \nFROM ${config.projects} \njoin ${config.solutions} on ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND tenant_id = ${tenant_id}\nAND ${config.solutions}.program_name is not null\n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}}\n AND tenant_id = ${tenant_id} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\nGROUP BY ${config.solutions}.program_name\nORDER BY \"Total Projects\" DESC;" |
There was a problem hiding this comment.
Fix total alias naming to match user aggregation.
Line 87 computes distinct users (created_by) but labels it as "Total Projects". Please rename to "Total Users" (and update ORDER BY) to avoid user-facing confusion.
💡 Proposed fix
-COUNT(DISTINCT ${config.projects}.created_by) AS "Total Projects" \nFROM ${config.projects} \njoin ${config.solutions} on ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND tenant_id = ${tenant_id}\nAND ${config.solutions}.program_name is not null\n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}}\n AND tenant_id = ${tenant_id} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\nGROUP BY ${config.solutions}.program_name\nORDER BY "Total Projects" DESC;"
+COUNT(DISTINCT ${config.projects}.created_by) AS "Total Users" \nFROM ${config.projects} \njoin ${config.solutions} on ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND tenant_id = ${tenant_id}\nAND ${config.solutions}.program_name is not null\n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}}\n AND tenant_id = ${tenant_id} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\nGROUP BY ${config.solutions}.program_name\nORDER BY "Total Users" DESC;"📝 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.
Suggested change
| "query": "SELECT DISTINCT(${config.solutions}.program_name) AS \"Programs\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'started' THEN ${config.projects}.created_by END) AS \"Started Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'inProgress' THEN ${config.projects}.created_by END) AS \"In Progress Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'submitted' THEN ${config.projects}.created_by END) AS \"Submitted Users\", \nCOUNT(DISTINCT ${config.projects}.created_by) AS \"Total Projects\" \nFROM ${config.projects} \njoin ${config.solutions} on ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND tenant_id = ${tenant_id}\nAND ${config.solutions}.program_name is not null\n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}}\n AND tenant_id = ${tenant_id} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\nGROUP BY ${config.solutions}.program_name\nORDER BY \"Total Projects\" DESC;" | |
| "query": "SELECT DISTINCT(${config.solutions}.program_name) AS \"Programs\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'started' THEN ${config.projects}.created_by END) AS \"Started Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'inProgress' THEN ${config.projects}.created_by END) AS \"In Progress Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'submitted' THEN ${config.projects}.created_by END) AS \"Submitted Users\", \nCOUNT(DISTINCT ${config.projects}.created_by) AS \"Total Users\" \nFROM ${config.projects} \njoin ${config.solutions} on ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND tenant_id = ${tenant_id}\nAND ${config.solutions}.program_name is not null\n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}}\n AND tenant_id = ${tenant_id} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\nGROUP BY ${config.solutions}.program_name\nORDER BY \"Total Users\" DESC;" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/graph/2_status_of_user_program_wise.json`
at line 87, The query labels COUNT(DISTINCT ${config.projects}.created_by) as
"Total Projects" which is incorrect since it counts distinct users; rename that
alias to "Total Users" and update the ORDER BY clause from ORDER BY "Total
Projects" DESC to ORDER BY "Total Users" DESC so the label and sort match the
${config.projects}.created_by aggregation.
| } | ||
| }, | ||
| "query": "SELECT DISTINCT(${config.projects}.state_name) AS \"State\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'started' THEN ${config.projects}.created_by END) AS \"Started Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'inProgress' THEN ${config.projects}.created_by END) AS \"In Progress Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'submitted' THEN ${config.projects}.created_by END) AS \"Submitted Users\", \nCOUNT(DISTINCT ${config.projects}.created_by) AS \"Total Users\" \nFROM ${config.projects} \njoin ${config.solutions} on ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND ${config.projects}.state_name is not null AND ${config.projects}.state_name <> ''\n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n LIMIT 1\n )\n ]]\nGROUP BY ${config.projects}.state_name\nORDER BY \"Total Users\" DESC;" | ||
| "query": "SELECT DISTINCT(${config.projects}.state_name) AS \"State\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'started' THEN ${config.projects}.created_by END) AS \"Started Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'inProgress' THEN ${config.projects}.created_by END) AS \"In Progress Users\", \nCOUNT(DISTINCT CASE WHEN ${config.projects}.status = 'submitted' THEN ${config.projects}.created_by END) AS \"Submitted Users\", \nCOUNT(DISTINCT ${config.projects}.created_by) AS \"Total Users\" \nFROM ${config.projects} \njoin ${config.solutions} on ${config.projects}.solution_id = ${config.solutions}.solution_id\nWHERE 1 = 1\nAND tenant_id = ${tenant_id}\nAND ${config.projects}.state_name is not null AND ${config.projects}.state_name <> ''\n[[AND ${config.projects}.state_id = (\n SELECT state_id\n FROM ${config.projects}\n WHERE {{state_param}}\n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]]\n [[AND ${config.projects}.district_id = (\n SELECT district_id\n FROM ${config.projects}\n WHERE {{district_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1)\n ]] \n [[AND ${config.projects}.program_id = (\n SELECT program_id\n FROM ${config.projects}\n WHERE {{program_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1 )\n ]]\n[[AND ${config.projects}.block_id = (\n SELECT block_id\n FROM ${config.projects}\n WHERE {{block_param}}\n AND tenant_id = ${tenant_id} \n LIMIT 1\n )\n ]] \n [[AND ${config.projects}.cluster_id = (\n SELECT cluster_id\n FROM ${config.projects}\n WHERE {{cluster_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\n [[AND ${config.projects}.org_id = (\n SELECT org_id\n FROM ${config.projects}\n WHERE {{org_param}} \n AND tenant_id = ${tenant_id}\n LIMIT 1\n )\n ]]\nGROUP BY ${config.projects}.state_name\nORDER BY \"Total Users\" DESC;" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden -S '\$\{config\.solutions\}\.tenant_id|\$\{config\.projects\}\.tenant_id|JOIN \$\{config\.solutions\}' metabase-jobs/config-data-loader/projectJson/AdminRepository: ELEVATE-Project/data-pipeline
Length of output: 9802
Qualify tenant_id to ${config.projects}.tenant_id in the WHERE clause.
The unqualified tenant_id predicate after joining ${config.projects} with ${config.solutions} creates ambiguity. Explicitly qualify it: AND ${config.projects}.tenant_id = ${tenant_id} to ensure correct filtering and maintain consistency across the Metabase queries.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@metabase-jobs/config-data-loader/projectJson/Admin/User-Details/json/graph/3_status_of_user_state_wise.json`
at line 87, The WHERE clause uses an unqualified tenant_id causing ambiguity
after the join; change the predicate to explicitly reference the projects table
by replacing tenant_id = ${tenant_id} with ${config.projects}.tenant_id =
${tenant_id} (update predicates inside the main WHERE and any nested SELECTs
that use tenant_id in the subqueries), ensuring all tenant filters consistently
reference ${config.projects}.tenant_id and leaving other table references
unchanged.
| "database": 2, | ||
| "type": "native", | ||
| "native": { | ||
| "query": "SELECT distinct(org_name) \nFROM ${config.projects} \nWHERE tenant_id = 'TENANTID'", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden -S 'TENANTID|\$\{tenant_id\}|select_organisation|UpdateAndAddAdminTenantFilter|ProcessAdminConstructor' metabase-jobsRepository: ELEVATE-Project/data-pipeline
Length of output: 50385
Use ${tenant_id} placeholder for consistency with the rest of the pipeline.
This filter query hardcodes TENANTID, which relies on the UpdateAndAddAdminTenantFilter to replace it at runtime. While the replacement mechanism exists, this pattern is inconsistent with other queries in the pipeline (e.g., Admin and User detail queries) that use the ${tenant_id} placeholder. Standardize on ${tenant_id} to improve maintainability and reduce the risk of missing replacements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@metabase-jobs/config-data-loader/projectJson/Project/Project-Filters/json/admin-filter/select_organisation.json`
at line 9, The query in the "query" property currently hardcodes 'TENANTID'
which is inconsistent with other pipeline queries; update the SQL string used in
the query field (the SELECT distinct(org_name) FROM ${config.projects} ...
entry) to replace the literal 'TENANTID' with the ${tenant_id} placeholder so
the runtime substitution matches the rest of the pipeline and avoids reliance on
UpdateAndAddAdminTenantFilter to mutate a literal.
Comment on lines
+315
to
+328
| val filterQuery: String = s"SELECT config FROM $reportConfig WHERE report_name = 'Project-Filters' AND question_type = 'admin-filter'" | ||
| val filterResults: List[Map[String, Any]] = postgresUtil.fetchData(filterQuery) | ||
| val objectMapper = new ObjectMapper() | ||
| val slugNameToTenantIdFilterMap = mutable.Map[String, Int]() | ||
| for (result <- filterResults) { | ||
| val configString = result.get("config").map(_.toString).getOrElse("") | ||
| val configJson = objectMapper.readTree(configString) | ||
| val slugName = configJson.findValue("name").asText() | ||
| val tenantIdFilter: Int = UpdateAndAddAdminTenantFilter.updateAndAddFilter(metabaseUtil, configJson: JsonNode, s"$tenantId", mipCollectionId, databaseId, projects, solutions) | ||
| slugNameToTenantIdFilterMap(slugName) = tenantIdFilter | ||
| } | ||
| val immutableSlugNameToTenantIdFilterMap: Map[String, Int] = slugNameToTenantIdFilterMap.toMap | ||
| val parametersQuery: String = s"SELECT config FROM $reportConfig WHERE report_name = 'Project-Parameter' AND question_type = 'admin-parameter'" | ||
| UpdateParameters.UpdateAdminParameterFunction(metabaseUtil, parametersQuery, mipDashboardId, postgresUtil) | ||
| UpdateParameters.updateParameterFunction(metabaseUtil, postgresUtil, parametersQuery, immutableSlugNameToTenantIdFilterMap, mipDashboardId) |
There was a problem hiding this comment.
Fail fast when an admin filter card is not created.
UpdateAndAddAdminTenantFilter.updateAndAddFilter returns -1 on failure, but this loop still stores that value. UpdateParameters.updateParameterFunction will then write card_id = -1 into the dashboard parameters, leaving the admin filters unusable.
Proposed fix
val slugName = configJson.findValue("name").asText()
val tenantIdFilter: Int = UpdateAndAddAdminTenantFilter.updateAndAddFilter(metabaseUtil, configJson: JsonNode, s"$tenantId", mipCollectionId, databaseId, projects, solutions)
+ if (tenantIdFilter == -1) {
+ throw new IllegalStateException(s"Failed to create admin filter card for '$slugName'")
+ }
slugNameToTenantIdFilterMap(slugName) = tenantIdFilterAs per coding guidelines metabase-jobs/**: "Verify: - Metabase cards/filters mapping consistency".
📝 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.
Suggested change
| val filterQuery: String = s"SELECT config FROM $reportConfig WHERE report_name = 'Project-Filters' AND question_type = 'admin-filter'" | |
| val filterResults: List[Map[String, Any]] = postgresUtil.fetchData(filterQuery) | |
| val objectMapper = new ObjectMapper() | |
| val slugNameToTenantIdFilterMap = mutable.Map[String, Int]() | |
| for (result <- filterResults) { | |
| val configString = result.get("config").map(_.toString).getOrElse("") | |
| val configJson = objectMapper.readTree(configString) | |
| val slugName = configJson.findValue("name").asText() | |
| val tenantIdFilter: Int = UpdateAndAddAdminTenantFilter.updateAndAddFilter(metabaseUtil, configJson: JsonNode, s"$tenantId", mipCollectionId, databaseId, projects, solutions) | |
| slugNameToTenantIdFilterMap(slugName) = tenantIdFilter | |
| } | |
| val immutableSlugNameToTenantIdFilterMap: Map[String, Int] = slugNameToTenantIdFilterMap.toMap | |
| val parametersQuery: String = s"SELECT config FROM $reportConfig WHERE report_name = 'Project-Parameter' AND question_type = 'admin-parameter'" | |
| UpdateParameters.UpdateAdminParameterFunction(metabaseUtil, parametersQuery, mipDashboardId, postgresUtil) | |
| UpdateParameters.updateParameterFunction(metabaseUtil, postgresUtil, parametersQuery, immutableSlugNameToTenantIdFilterMap, mipDashboardId) | |
| val filterQuery: String = s"SELECT config FROM $reportConfig WHERE report_name = 'Project-Filters' AND question_type = 'admin-filter'" | |
| val filterResults: List[Map[String, Any]] = postgresUtil.fetchData(filterQuery) | |
| val objectMapper = new ObjectMapper() | |
| val slugNameToTenantIdFilterMap = mutable.Map[String, Int]() | |
| for (result <- filterResults) { | |
| val configString = result.get("config").map(_.toString).getOrElse("") | |
| val configJson = objectMapper.readTree(configString) | |
| val slugName = configJson.findValue("name").asText() | |
| val tenantIdFilter: Int = UpdateAndAddAdminTenantFilter.updateAndAddFilter(metabaseUtil, configJson: JsonNode, s"$tenantId", mipCollectionId, databaseId, projects, solutions) | |
| if (tenantIdFilter == -1) { | |
| throw new IllegalStateException(s"Failed to create admin filter card for '$slugName'") | |
| } | |
| slugNameToTenantIdFilterMap(slugName) = tenantIdFilter | |
| } | |
| val immutableSlugNameToTenantIdFilterMap: Map[String, Int] = slugNameToTenantIdFilterMap.toMap | |
| val parametersQuery: String = s"SELECT config FROM $reportConfig WHERE report_name = 'Project-Parameter' AND question_type = 'admin-parameter'" | |
| UpdateParameters.updateParameterFunction(metabaseUtil, postgresUtil, parametersQuery, immutableSlugNameToTenantIdFilterMap, mipDashboardId) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@metabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/functions/ProjectMetabaseDashboardFunction.scala`
around lines 315 - 328, The loop stores a -1 return from
UpdateAndAddAdminTenantFilter.updateAndAddFilter on failure, which later causes
bad card_id values; change the loop to fail-fast: after calling
UpdateAndAddAdminTenantFilter.updateAndAddFilter (referenced symbol), check if
tenantIdFilter == -1 and if so throw a RuntimeException (or otherwise abort)
with a clear message including slugName and configString; only insert into
slugNameToTenantIdFilterMap when tenantIdFilter != -1 so
UpdateParameters.updateParameterFunction receives only valid card IDs.
Comment on lines
+74
to
+87
| def getTheQuestionId(json: JsonNode): Int = { | ||
| try { | ||
| val requestBody = json.get("questionCard") | ||
| val questionCardResponse = metabaseUtil.createQuestionCard(requestBody.toString) | ||
| val responseJson = objectMapper.readTree(questionCardResponse) | ||
| Option(responseJson.get("id")).map(_.asInt()).getOrElse { | ||
| println("Error: 'id' field not found in the response.") | ||
| -1 | ||
| } | ||
| } catch { | ||
| case ex: Exception => | ||
| println(s"Error fetching 'id' from response: ${ex.getMessage}") | ||
| -1 | ||
| } |
There was a problem hiding this comment.
Do not return -1 for card creation failures.
-1 is treated as a real card_id by the caller, so any Metabase create/parse failure will silently poison the tenant filter map and break downstream parameter wiring. Fail fast here, or return an Option[Int]/Either and let the caller skip the slug on failure. As per coding guidelines: Metabase cards/filters mapping consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@metabase-jobs/project-dashboard-creator/src/main/scala/org/shikshalokam/job/dashboard/creator/functions/UpdateAndAddAdminTenantFilter.scala`
around lines 74 - 87, The getTheQuestionId function currently returns -1 on
failures which is interpreted as a valid card_id downstream; change
getTheQuestionId to fail-fast or return an explicit failure type (e.g.,
Option[Int] or Either[Throwable, Int]) instead of -1 so callers can skip or
handle missing cards; update the method signature (getTheQuestionId) and replace
the printlns with proper error propagation (throw the caught exception or return
None/Left with the error), and ensure callers handle the new return type to
avoid poisoning the tenant filter map.
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
1 participant
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.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor