Rename Credential to work history #3344
Conversation
shubham1g5
requested review from
Jignesh-dimagi,
OrangeAndGreen and
jaypanchal-13
📝 WalkthroughWalkthroughRenames and repurposes the Personal ID “Credential” feature to “Work History” across the app. Updates AndroidManifest to launch PersonalIdWorkHistoryActivity. Replaces model (PersonalIdCredential → PersonalIdWorkHistory), ViewModel, adapters, fragments, and ViewPager adapter. Updates database helpers/migrations to the new table/model. Revises Connect navigation target and nav drawer label. Changes layouts, IDs, menus, icons, and string resources (en/es/fr/hi/pt/sw/ti) to the work_history namespace. Removes PendingCredentialAdapter and adds WorkHistoryPendingAdapter. Adjusts StringUtils level string mappings. Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Drawer as Nav Drawer
participant Activity as PersonalIdWorkHistoryActivity
participant VM as PersonalIdWorkHistoryViewModel
participant API as RetrieveCredentialsResponseParser
participant DB as Connect DB
participant Earned as WorkHistoryEarnedFragment
participant Pending as WorkHistoryPendingFragment
participant Adapters as RecyclerView Adapters
User->>Drawer: Tap "Work History"
Drawer->>Activity: startActivity(...)
Activity->>VM: observeWorkHistoryApiCall()\nretrieveAndProcessWorkHistory()
VM->>API: fetchWorkHistoryFromNetwork()
API->>DB: parse JSON → List<PersonalIdWorkHistory>\nstoreCredentialDataInTable(...)
API-->>VM: success(List<PersonalIdWorkHistory>)
VM->>VM: compute earned/pending
VM-->>Earned: post earnedWorkHistory (LiveData)
VM-->>Pending: post pendingWorkHistory (LiveData)
Earned->>Adapters: WorkHistoryEarnedAdapter.setData(...)
Pending->>Adapters: WorkHistoryPendingAdapter.setData(...)
note over Earned,Pending: Toggle empty-state views\n(rv*/tvNo*)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/src/org/commcare/android/database/connect/models/PersonalIdWorkHistory.kt (1)
56-58: STORAGE_KEY must match legacy table name ("personal_id_credential") or provide a DB migration.Database history and tests reference "personal_id_credential" (DatabaseConnectOpenHelper comments, ConnectDatabaseUpgrader, unit-tests). Current STORAGE_KEY = "credential" will create a new table and cause schema drift.
Location: app/src/org/commcare/android/database/connect/models/PersonalIdWorkHistory.kt (companion object STORAGE_KEY)
- const val STORAGE_KEY = "credential" + const val STORAGE_KEY = "personal_id_credential" // match legacy table nameapp/src/org/commcare/models/database/connect/ConnectDatabaseUpgrader.java (1)
638-647: Fix nested transactions in upgradeSeventeenEighteen (likely crash during DB upgrade).upgradeSeventeenEighteen starts a transaction and then calls addTableForNewModel, which itself opens a transaction. Our DB wrapper does not support nested transactions; this can throw at runtime during upgrade to version 18.
Apply this diff to remove the outer transaction and rely on the transactional boundaries of dropTable/addTableForNewModel:
- private void upgradeSeventeenEighteen(IDatabase db) { - db.beginTransaction(); - try { - // We have not been populating this table yet, so just drop and recreate - SqlStorage.dropTable(db, PersonalIdWorkHistory.STORAGE_KEY); - addTableForNewModel(db, PersonalIdWorkHistory.STORAGE_KEY, new PersonalIdWorkHistory()); - } finally { - db.endTransaction(); - } - } + private void upgradeSeventeenEighteen(IDatabase db) { + // We have not been populating this table yet, so just drop and recreate + SqlStorage.dropTable(db, PersonalIdWorkHistory.STORAGE_KEY); + addTableForNewModel(db, PersonalIdWorkHistory.STORAGE_KEY, new PersonalIdWorkHistory()); + }app/src/org/commcare/adapters/WorkHistoryEarnedAdapter.kt (1)
30-41: Click listener is not hooked; items are not actionable.As with the pending adapter, the listener isn’t used.
Apply:
override fun onBindViewHolder(holder: WorkHistoryViewHolder, position: Int) { val item = workHistoryList[position] val formattedIssuedDate: String = convertIsoDate(item.issuedDate, "dd/MM/yyyy") with(holder.binding) { tvAppName.text = item.title tvIssuedDate.text = formattedIssuedDate tvActivity.text = StringUtils.getLocalizedLevel(item.level, holder.itemView.context) Glide.with(ivProfilePic).load(profilePic).into(ivProfilePic) } + holder.binding.root.setOnClickListener { + val pos = holder.bindingAdapterPosition + if (pos != RecyclerView.NO_POSITION) { + listener.onWorkHistoryItemClick(workHistoryList[pos]) + } + } }app/src/org/commcare/activities/connect/viewmodel/PersonalIdWorkHistoryViewModel.kt (1)
81-94: Tear down per-app sandboxes to prevent handle/file leaksEach iteration seats a new sandbox; ensure it’s torn down before moving on.
private fun getWorkHistoryFromAppRecords(records: ArrayList<ApplicationRecord>): List<PersonalIdWorkHistory> { - return records.flatMap { record -> - val commcareApp = CommCareApp(record) - commcareApp.setupSandbox() - val profile = commcareApp.initApplicationProfile(); - profile.credentials.map { credential -> - PersonalIdWorkHistory().apply { - appId = record.applicationId - title = record.displayName ?: "" - level = credential.level - } - } - } + return records.flatMap { record -> + val app = CommCareApp(record) + try { + app.setupSandbox() + val profile = app.initApplicationProfile() + profile.credentials.map { credential -> + PersonalIdWorkHistory().apply { + appId = record.applicationId + title = record.displayName ?: "" + level = credential.level + // Consider copying a stable key if available (e.g., slug) to avoid locale mismatches. + } + } + } finally { + app.teardownSandbox() + } + } }
🧹 Nitpick comments (31)
app/res/layout/activity_personal_id_credential.xml (1)
47-47: Text updated to Work History; consider file/ID renames for consistency.OK as-is. When convenient, consider renaming this layout and IDs (e.g.,
tab_credentials→tab_work_history) to match the new terminology.app/src/org/commcare/navdrawer/BaseDrawerController.kt (1)
191-197: Mismatch: "Work History" label uses NavItemType.CREDENTIAL — fix before enabling. BaseDrawerController creates a ParentItem with R.string.personalid_work_history but assigns NavItemType.CREDENTIAL (app/src/org/commcare/navdrawer/BaseDrawerController.kt:193–197); BaseDrawerActivity.handleDrawerItemClick treats WORK_HISTORY as a no-op and CREDENTIAL -> navigateToCredential (app/src/org/commcare/navdrawer/BaseDrawerActivity.kt:77–86). Use NavItemType.WORK_HISTORY here or remove/merge the redundant enum and update handlers; keep the feature behind shouldShowCredential() until resolved.app/src/org/commcare/connect/ConnectNavHelper.kt (1)
9-10: Navigation target switched to PersonalIdWorkHistoryActivity — approved.Verified the activity class and AndroidManifest entry; goToCredentials(context) now starts PersonalIdWorkHistoryActivity.
Optional: rename goToCredentials → goToWorkHistory in ConnectNavHelper.kt for clarity (non-blocking).app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java (1)
59-62: Version history comment likely outdated.Comments still say “personal_id_credential table.” If the STORAGE_KEY stays “credential,” update the doc to avoid confusion.
app/src/org/commcare/connect/database/ConnectAppDatabaseUtil.java (1)
69-77: Method naming + full-table wipe: confirm intent and consider scoping.
- The method still reads “Credential” and param
validCredentials; consider renaming to “WorkHistory” for clarity.storage.removeAll()wipes the entire table. If multi‑user or multi‑app data shares this table, this will purge others’ data. If that’s unintended, filter deletions by user/app.Proposed cleanups:
- public static void storeCredentialDataInTable(Context context, List<PersonalIdWorkHistory> validCredentials) { - SqlStorage<PersonalIdWorkHistory> storage = - ConnectDatabaseHelper.getConnectStorage(context, PersonalIdWorkHistory.class); + public static void storeWorkHistoryDataInTable(Context context, List<PersonalIdWorkHistory> records) { + SqlStorage<PersonalIdWorkHistory> storage = + ConnectDatabaseHelper.getConnectStorage(context, PersonalIdWorkHistory.class); storage.removeAll(); - for (PersonalIdWorkHistory credential : validCredentials) { - storage.write(credential); + for (PersonalIdWorkHistory r : records) { + storage.write(r); } }If you adopt the rename, update callers (e.g.,
RetrieveCredentialsResponseParser) accordingly.app/res/layout/fragment_work_history_earned.xml (1)
25-37: RecyclerView nits: remove unused orientation; update tools list item if available.
android:orientationon RecyclerView is ignored; LM controls orientation.- If a
@layout/item_earned_work_historyexists, prefer it for preview.- android:orientation="vertical" - tools:listitem="@layout/item_earned_credential" /> + tools:listitem="@layout/item_earned_work_history" />app/res/menu/menu_work_history.xml (1)
11-16: “More” menu item title points to “Sync”.Likely copy‑paste. Title and contentDescription should reflect “More/Overflow”.
<item android:id="@+id/more" - android:title="@string/personalid_work_history_sync" - android:contentDescription="action" + android:title="@string/personalid_work_history_more" + android:contentDescription="@string/personalid_work_history_more" android:icon="@drawable/ic_connect_action_overflow" app:showAsAction="always"/>Add
personalid_work_history_moreto strings if missing.app/res/layout/fragment_work_history_pending.xml (1)
22-35: RecyclerView nits and preview layout.
- Remove unused
android:orientation.- If available, point preview to
item_pending_work_history.- android:orientation="vertical" - tools:listitem="@layout/item_pending_credential" /> + tools:listitem="@layout/item_pending_work_history" />app/src/org/commcare/activities/connect/PersonalIdWorkHistoryActivity.kt (2)
30-38: Avoid NPE on username; prefer safe access.
userName!!can crash if VM hasn’t populated yet. Also prefer safe access onprofilePic.- userName = personalIdWorkHistoryViewModel.userName - profilePic = personalIdWorkHistoryViewModel.profilePhoto - workHistoryViewPagerAdapter = WorkHistoryViewPagerAdapter(this,userName!!,profilePic ?: "") + userName = personalIdWorkHistoryViewModel.userName + profilePic = personalIdWorkHistoryViewModel.profilePhoto + val u = userName + if (u.isNullOrEmpty()) { + Toast.makeText(this, R.string.personalid_work_history_missing_username, Toast.LENGTH_SHORT).show() + finish() + return + } + workHistoryViewPagerAdapter = WorkHistoryViewPagerAdapter(this, u, profilePic.orEmpty())
41-47: Null‑safe ActionBar access.
supportActionBar!!will crash on null. Use safe call.- supportActionBar!!.apply { + supportActionBar?.apply { title = getString(R.string.personalid_work_history_title) setDisplayShowHomeEnabled(true) setDisplayHomeAsUpEnabled(true) }app/src/org/commcare/android/database/connect/models/PersonalIdWorkHistory.kt (1)
92-97: Update log message to match new domain.Keep logs consistent with “Work History”.
- val errorMessage = "Corrupt credential at index $i: $corruptedJson" + val errorMessage = "Corrupt work history record at index $i: $corruptedJson"app/src/org/commcare/models/database/connect/ConnectDatabaseUpgrader.java (1)
601-603: LGTM: table creation pivots to the new model while keeping the same STORAGE_KEY.Keeps on-disk schema name stable while changing the model class. Consider adding a short comment noting that STORAGE_KEY intentionally remains "credential" for backward compatibility.
app/src/org/commcare/adapters/WorkHistoryPendingAdapter.kt (1)
38-42: Prefer DiffUtil/ListAdapter for smoother updates.notifyDataSetChanged() rebinds everything and can cause jank. Consider ListAdapter with DiffUtil.ItemCallback.
app/src/org/commcare/adapters/WorkHistoryEarnedAdapter.kt (2)
12-15: Minor constructor formatting nit.Add a space after the comma for readability or move params onto separate lines.
45-49: Use DiffUtil/ListAdapter for incremental updates.Avoid notifyDataSetChanged() to improve scroll perf and enable item animations.
app/res/values-fr/strings.xml (1)
474-487: French copy: align with “Work History” semantics and French casing; remove forced line breaks.
- “En Savoir Plus Sur les Titres de Compétences” still refers to credentials and uses title case.
- “Historique de Travail” should be “Historique de travail”.
- Level strings don’t need explicit “\n”; let layout wrap.
Apply:
- <string name="personalid_work_history_title">Mon Historique de Travail</string> - <string name="personalid_work_history">Historique de Travail</string> - <string name="personalid_work_history_learn_more">En Savoir Plus Sur les Titres de Compétences</string> + <string name="personalid_work_history_title">Mon historique de travail</string> + <string name="personalid_work_history">Historique de travail</string> + <string name="personalid_work_history_learn_more">En savoir plus sur l’historique professionnel</string> - <string name="personalid_work_history_level_1_month_active">1 Mois d'\nActivité</string> - <string name="personalid_work_history_level_2_month_active">2 Mois d'\nActivité</string> - <string name="personalid_work_history_level_3_month_active">3 Mois d'\nActivité</string> - <string name="personalid_work_history_level_6_month_active">6 Mois d'\nActivité</string> - <string name="personalid_work_history_level_9_month_active">9 Mois d'\nActivité</string> - <string name="personalid_work_history_level_12_month_active">12 Mois d'\nActivité</string> + <string name="personalid_work_history_level_1_month_active">1 mois d’activité</string> + <string name="personalid_work_history_level_2_month_active">2 mois d’activité</string> + <string name="personalid_work_history_level_3_month_active">3 mois d’activité</string> + <string name="personalid_work_history_level_6_month_active">6 mois d’activité</string> + <string name="personalid_work_history_level_9_month_active">9 mois d’activité</string> + <string name="personalid_work_history_level_12_month_active">12 mois d’activité</string>app/src/org/commcare/fragments/personalId/WorkHistoryPendingFragment.kt (3)
39-46: No-op click handler; either implement navigation or remove listener until used.onWorkHistoryItemClick is empty. Hook this to details/navigation, or pass a no-op listener from the caller to keep responsibilities clear.
60-66: Unused argument (username).ARG_USERNAME is read but not used. If not needed, drop the arg to reduce confusion. If it’s for filtering, add the filter to the ViewModel query.
33-35: Avoid force-unwrapping binding with !!.Use a local val and safe calls to prevent accidental NPEs during lifecycle edge cases.
- binding = FragmentWorkHistoryPendingBinding.inflate(inflater, container, false) - return binding!!.root + val b = FragmentWorkHistoryPendingBinding.inflate(inflater, container, false) + binding = b + return b.root ... - binding!!.rvPendingWorkHistory.adapter = workHistoryPendingAdapter + binding?.rvPendingWorkHistory?.adapter = workHistoryPendingAdapterAlso applies to: 45-46, 71-72
app/res/values-sw/strings.xml (1)
482-482: Update “Learn more” text to match Work HistoryThe label still says “Vitambulisho” (Credentials). Suggest switching to Work History to avoid mixed terminology.
- <string name="personalid_work_history_learn_more">Jifunze Zaidi Kuhusu Vitambulisho</string> + <string name="personalid_work_history_learn_more">Jifunze zaidi kuhusu Historia ya Kazi</string>app/src/org/commcare/fragments/personalId/WorkHistoryEarnedFragment.kt (3)
14-18: Avoid!!with a proper view-binding patternUse a backing property to eliminate
!!and make lifecycle-safe access.-class WorkHistoryEarnedFragment : Fragment() { - private var binding: FragmentWorkHistoryEarnedBinding? = null +class WorkHistoryEarnedFragment : Fragment() { + private var _binding: FragmentWorkHistoryEarnedBinding? = null + private val binding get() = _binding!! @@ - binding = FragmentWorkHistoryEarnedBinding.inflate(inflater, container, false) - return binding!!.root + _binding = FragmentWorkHistoryEarnedBinding.inflate(inflater, container, false) + return binding.root @@ - binding!!.rvEarnedWorkHistory.adapter = workHistoryEarnedAdapter + binding.rvEarnedWorkHistory.adapter = workHistoryEarnedAdapter @@ - binding!!.tvNoWorkHistoryAvailable.visibility = View.VISIBLE + binding.tvNoWorkHistoryAvailable.visibility = View.VISIBLE @@ - binding!!.tvNoWorkHistoryAvailable.visibility = View.GONE + binding.tvNoWorkHistoryAvailable.visibility = View.GONE @@ - binding = null + _binding = nullAlso applies to: 33-35, 47-47, 51-56, 73-76
39-43: Empty click handler (detekt: EmptyFunctionBlock)Either implement an action or add a no-op statement to appease the linter.
override fun onWorkHistoryItemClick(workHistory: PersonalIdWorkHistory) { - + // Intentionally empty for now; implement navigation later + Unit }
49-56: Null-safety and visibility toggling for the listUse
orEmpty()and toggle RecyclerView visibility along with the empty view.- viewModel.earnedWorkHistory.observe(viewLifecycleOwner) { earnedList -> - if (earnedList.isNullOrEmpty()) { - binding!!.tvNoWorkHistoryAvailable.visibility = View.VISIBLE - } else { - binding!!.tvNoWorkHistoryAvailable.visibility = View.GONE - } - workHistoryEarnedAdapter.setData(earnedList) - } + viewModel.earnedWorkHistory.observe(viewLifecycleOwner) { earnedList -> + val data = earnedList.orEmpty() + binding.tvNoWorkHistoryAvailable.visibility = + if (data.isEmpty()) View.VISIBLE else View.GONE + binding.rvEarnedWorkHistory.visibility = + if (data.isEmpty()) View.GONE else View.VISIBLE + workHistoryEarnedAdapter.setData(data) + }app/res/values-ti/strings.xml (2)
467-468: Inconsistent terminology and untranslated string
- “Learn more” still references credentials instead of work history.
- “No work history records...” is in English.
Proposed fixes (please verify with your TI translator/localization pipeline):
- <string name="personalid_work_history_learn_more">ብዛዕባ ምስክር ወረቐታት ዝበለጸ ፍለጥ</string> + <string name="personalid_work_history_learn_more">ብዛዕባ ታሪኽ ስራሕ ዝበለጸ ፍለጥ</string> - <string name="personalid_work_history_no_records_available">No work history records are currently available. Click Refresh or check back later</string> + <string name="personalid_work_history_no_records_available">ብሕጂ መዝገቦ ታሪኽ ስራሕ የለን። ኣሐድስ ጠውቕ ወይ ድማ ድሕሪ ግዜ ተመለስ።</string>Also consider localizing:
- <string name="personalid_work_history_sync">Sync</string> + <string name="personalid_work_history_sync">ምትእስሳር</string>
481-481: Untranslated pending messageEnglish fallback remains. Please localize or confirm intended fallback.
Suggested TI (please validate):
- <string name="personalid_work_history_no_pending_records">There are no pending records at the moment</string> + <string name="personalid_work_history_no_pending_records">ብሕጂ መዝገቦ ኣብ ከይዲ የለን</string>app/src/org/commcare/activities/connect/viewmodel/PersonalIdWorkHistoryViewModel.kt (3)
49-56: Pending-list diffing may break on localized titlesMatching on
titleis fragile across locales. Prefer stable keys (e.g.,slug,oppId) plusappId/level.If
profile.credentialsexposes a slug or ID, populate it ingetWorkHistoryFromAppRecords(...)and switch the predicate:- val pending = installedAppsWorkHistory.filter { installedWorkHistory -> - !earned.any { earnedWorkHistory -> - earnedWorkHistory.appId == installedWorkHistory.appId && - earnedWorkHistory.title == installedWorkHistory.title && - earnedWorkHistory.level == installedWorkHistory.level - } - } + val pending = installedAppsWorkHistory.filter { installed -> + !earned.any { e -> + e.appId == installed.appId && + e.level == installed.level && + e.slug == installed.slug // or oppId if more appropriate + } + }
31-32: Lateinit for list is unnecessary; simplify initializationInitialize to
emptyList()and drop theisInitializedcheck.- private lateinit var installedAppsWorkHistory : List<PersonalIdWorkHistory> + private var installedAppsWorkHistory: List<PersonalIdWorkHistory> = emptyList() @@ - if (!::installedAppsWorkHistory.isInitialized) { + if (installedAppsWorkHistory.isEmpty()) { installedAppsWorkHistory = evalInstalledAppsWorkHistory() }Also applies to: 46-48
37-43: API method name mismatch (nit)Method is
retrieveAndProcessWorkHistory()but callsretrieveCredentials(...). If the API is truly about work history now, consider renaming the API method for clarity (or add a comment).Also applies to: 67-68
app/res/values-es/strings.xml (1)
474-474: Align “Learn more” with Work History terminologyReplace “Credenciales” with “historial laboral” to avoid mixing concepts.
- <string name="personalid_work_history_learn_more">Más Información Sobre las Credenciales</string> + <string name="personalid_work_history_learn_more">Más información sobre el historial laboral</string>app/res/values-hi/strings.xml (1)
472-472: Update “Learn more” copy to match the rename (Credentials → Work History).The Hindi string still mentions “क्रेडेंशियल्स”. Suggest aligning with Work History.
Apply this diff:
- <string name="personalid_work_history_learn_more">क्रेडेंशियल्स के बारे में और जानें</string> + <string name="personalid_work_history_learn_more">कार्य इतिहास के बारे में और जानें</string>app/res/values-pt/strings.xml (1)
482-482: Align PT “Learn more” with Work History (not Credentials).Change “Credenciais” to “Histórico de Trabalho” for consistency.
Apply this diff:
- <string name="personalid_work_history_learn_more">Saiba Mais Sobre Credenciais</string> + <string name="personalid_work_history_learn_more">Saiba mais sobre o Histórico de Trabalho</string>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
app/AndroidManifest.xml(1 hunks)app/res/layout/activity_personal_id_credential.xml(1 hunks)app/res/layout/fragment_work_history_earned.xml(3 hunks)app/res/layout/fragment_work_history_pending.xml(2 hunks)app/res/layout/item_earned_credential.xml(1 hunks)app/res/layout/item_pending_credential.xml(2 hunks)app/res/menu/menu_work_history.xml(1 hunks)app/res/values-es/strings.xml(1 hunks)app/res/values-fr/strings.xml(1 hunks)app/res/values-hi/strings.xml(2 hunks)app/res/values-pt/strings.xml(1 hunks)app/res/values-sw/strings.xml(1 hunks)app/res/values-ti/strings.xml(1 hunks)app/res/values/strings.xml(2 hunks)app/src/org/commcare/activities/connect/PersonalIdWorkHistoryActivity.kt(2 hunks)app/src/org/commcare/activities/connect/viewmodel/PersonalIdWorkHistoryViewModel.kt(3 hunks)app/src/org/commcare/adapters/PendingCredentialAdapter.kt(0 hunks)app/src/org/commcare/adapters/WorkHistoryEarnedAdapter.kt(2 hunks)app/src/org/commcare/adapters/WorkHistoryPendingAdapter.kt(1 hunks)app/src/org/commcare/adapters/WorkHistoryViewPagerAdapter.kt(2 hunks)app/src/org/commcare/android/database/connect/models/PersonalIdWorkHistory.kt(4 hunks)app/src/org/commcare/connect/ConnectNavHelper.kt(2 hunks)app/src/org/commcare/connect/database/ConnectAppDatabaseUtil.java(2 hunks)app/src/org/commcare/connect/network/connectId/parser/RetrieveCredentialsResponseParser.kt(2 hunks)app/src/org/commcare/fragments/personalId/WorkHistoryEarnedFragment.kt(2 hunks)app/src/org/commcare/fragments/personalId/WorkHistoryPendingFragment.kt(2 hunks)app/src/org/commcare/models/database/connect/ConnectDatabaseUpgrader.java(3 hunks)app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java(2 hunks)app/src/org/commcare/navdrawer/BaseDrawerController.kt(1 hunks)app/src/org/commcare/utils/StringUtils.java(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/org/commcare/adapters/PendingCredentialAdapter.kt
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: shubham1g5
PR: dimagi/commcare-android#0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.
📚 Learning: 2025-05-08T11:08:18.530Z
Learnt from: shubham1g5
PR: dimagi/commcare-android#0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.
Applied to files:
app/src/org/commcare/connect/ConnectNavHelper.ktapp/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java
📚 Learning: 2025-01-21T17:28:09.007Z
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#2912
File: app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java:66-71
Timestamp: 2025-01-21T17:28:09.007Z
Learning: The ConnectUserRecord class in CommCare Android uses Persisting annotations with sequential indices for field persistence, and MetaField annotations for fields that have corresponding META constants. Fields should be documented with Javadoc comments explaining their purpose, format requirements, and relationships with other fields.
Applied to files:
app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.javaapp/src/org/commcare/models/database/connect/ConnectDatabaseUpgrader.java
🧬 Code graph analysis (5)
app/src/org/commcare/utils/StringUtils.java (1)
app/src/org/commcare/connect/network/connectId/parser/RetrieveCredentialsResponseParser.kt (1)
context(13-23)
app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java (1)
app/src/org/commcare/android/database/connect/models/PersonalIdWorkHistory.kt (1)
PersonalIdWorkHistory(13-103)
app/src/org/commcare/connect/database/ConnectAppDatabaseUtil.java (3)
app/src/org/commcare/android/database/connect/models/PersonalIdWorkHistory.kt (1)
PersonalIdWorkHistory(13-103)app/src/org/commcare/connect/network/connectId/parser/RetrieveCredentialsResponseParser.kt (1)
context(13-23)app/src/org/commcare/connect/database/ConnectDatabaseHelper.java (1)
ConnectDatabaseHelper(37-138)
app/src/org/commcare/activities/connect/viewmodel/PersonalIdWorkHistoryViewModel.kt (3)
app/src/org/commcare/connect/ConnectDateUtils.kt (1)
parseIsoDateForSorting(50-59)app/src/org/commcare/CommCareApp.java (1)
CommCareApp(58-473)app/src/org/commcare/android/database/connect/models/PersonalIdWorkHistory.kt (1)
PersonalIdWorkHistory(13-103)
app/src/org/commcare/models/database/connect/ConnectDatabaseUpgrader.java (2)
app/src/org/commcare/android/database/connect/models/PersonalIdWorkHistory.kt (1)
PersonalIdWorkHistory(13-103)app/src/org/commcare/models/database/SqlStorage.java (1)
SqlStorage(38-799)
🪛 detekt (1.23.8)
app/src/org/commcare/fragments/personalId/WorkHistoryEarnedFragment.kt
[warning] 41-43: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
🔇 Additional comments (12)
app/src/org/commcare/utils/StringUtils.java (1)
56-62: Level mappings updated to Work History strings — good change. Verified with the provided script: no missing-key reports — all six keys (personalid_work_history_level_{1,2,3,6,9,12}_month_active) are present in the scanned values/strings.xml files.app/src/org/commcare/connect/network/connectId/parser/RetrieveCredentialsResponseParser.kt (1)
4-5: Confirmed: storage signature updated to List and parser call is aligned.storeCredentialDataInTable signature:
public static void storeCredentialDataInTable(Context context, List<PersonalIdWorkHistory> validCredentials)— app/src/org/commcare/connect/database/ConnectAppDatabaseUtil.java:69.
RetrieveCredentialsResponseParser builds the list viaPersonalIdWorkHistory.fromJsonArray(jsonArray)and callsstoreCredentialDataInTable(context, result)— app/src/org/commcare/connect/network/connectId/parser/RetrieveCredentialsResponseParser.kt:19–21. No other usages ofstoreCredentialDataInTablefound in app/src.app/res/layout/item_earned_credential.xml (1)
84-84: Drawable rename OK — verify asset exists and fix "ribbin" typoConfirm ic_personalid_work_history_ribbin exists in the appropriate drawable density folders (or as a vector) and rename to ic_personalid_work_history_ribbon, updating any usages.
File: app/res/layout/item_earned_credential.xml — android:src="@drawable/ic_personalid_work_history_ribbin"
app/AndroidManifest.xml (1)
569-571: Manifest: single PersonalIdWorkHistoryActivity entry — verify attributes.
app/AndroidManifest.xml contains one entry: (lines 569–570); no intent-filter so exported not required (defaults to false on API 31+). Confirm the activity doesn't rely on attributes from the prior entry (theme, orientation).app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java (1)
23-23: Import rename looks good.Matches the new model usage.
app/src/org/commcare/adapters/WorkHistoryViewPagerAdapter.kt (1)
6-7: Adapter rename and fragment wiring verified.
Two tabs preserved; fragment factory signatures match adapter usage (WorkHistoryEarnedFragment.newInstance(username, profilePic) and WorkHistoryPendingFragment.newInstance(username)).app/src/org/commcare/fragments/personalId/WorkHistoryEarnedFragment.kt (1)
63-65: Factory method rename looks goodConsistent with the fragment rename and call sites.
app/res/values-pt/strings.xml (1)
480-494: PT Work History block looks consistent; verify no old credential_ usages remain.*Content reads fine after the copy fix. Please validate there are no remaining references to removed credential keys.
Use the same repo-wide scan as suggested in the HI file comment.
app/res/values/strings.xml (3)
647-654: LGTM: English Work History strings added correctly.
667-674: LGTM: Activity level and empty-state strings look consistent with prior pattern.
647-674: No stale personalid_credential_ references found; personalid_work_history_ keys present and referenced.**rg across app/ returned no matches for "personalid_credential_". personalid_work_history_* strings exist in app/res/values/strings.xml and are referenced in code/layouts/menus and localized resources (e.g. app/src/org/commcare/activities/connect/PersonalIdWorkHistoryActivity.kt; app/src/org/commcare/utils/StringUtils.java; app/res/menu/menu_work_history.xml; app/res/layout/item_pending_credential.xml, fragment_work_history_pending.xml, fragment_work_history_earned.xml, activity_personal_id_credential.xml; translations in app/res/values-*/strings.xml).
app/res/values-hi/strings.xml (1)
470-486: Work History strings verified — no lingering personalid_credential_ references*No matches for "personalid_credential_" were found; personalid_work_history_* keys are referenced in app/res/menu/menu_work_history.xml, app/res/layout/activity_personal_id_credential.xml, app/res/layout/item_pending_credential.xml, app/src/org/commcare/utils/StringUtils.java, app/res/layout/fragment_work_history_*.xml, and app/src/org/commcare/activities/connect/PersonalIdWorkHistoryActivity.kt.
| android:src="@drawable/ic_personalid_work_history_pending_person" | ||
| app:layout_constraintEnd_toEndOf="@id/tvPending" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Drawable/string updates aligned; check locale coverage and plan resource naming cleanup.
Please verify @string/personalid_work_history_pending exists in all locale files and the new drawable is present. Optional: background/color resources still use personal_id_credential_*; consider mirroring under *_work_history later for consistency.
Check for missing translations:
Also applies to: 33-34
🏁 Script executed:
#!/bin/bash
# List any strings.xml missing the new key
fd -a 'values*/*strings.xml' app/res | while read -r f; do
if ! rg -q 'name="personalid_work_history_pending"' "$f"; then
echo "Missing in: $f"
fi
doneLength of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Working directory: $(pwd)"
echo
# Find all strings xml files under any res/values* directories
echo "Finding strings.xml files under */res/values* ..."
mapfile -t strings_files < <(find . -type f -path "*/res/values*/strings*.xml" -print)
if [ ${#strings_files[@]} -eq 0 ]; then
echo "No strings.xml files found under res/values*"
else
echo "Found ${#strings_files[@]} strings file(s):"
for f in "${strings_files[@]}"; do printf " - %s\n" "$f"; done
fi
echo
# Check presence of the string key
echo "Checking for name=\"personalid_work_history_pending\" in each strings file..."
present=0
missing=0
for f in "${strings_files[@]}"; do
if grep -q 'name="personalid_work_history_pending"' "$f"; then
echo "Present: $f"
present=$((present+1))
else
echo "Missing: $f"
missing=$((missing+1))
fi
done
echo "Summary: present=$present missing=$missing total=${#strings_files[@]}"
echo
# Search for exact drawable filename under res/drawable* directories
echo "Searching for drawable files named ic_personalid_work_history_pending_person.* under res/drawable* ..."
mapfile -t exact_drawables < <(find . -type f -path "*/res/drawable*" -iname "ic_personalid_work_history_pending_person.*" -print)
if [ ${#exact_drawables[@]} -gt 0 ]; then
echo "Exact drawable file(s) found:"
for d in "${exact_drawables[@]}"; do printf " - %s\n" "$d"; done
else
echo "No exact drawable filename match found. Doing fuzzy search for 'personalid_work_history_pending' in drawable dirs..."
mapfile -t fuzzy_drawables < <(find . -type f -path "*/res/drawable*" -iname "*personalid_work_history_pending*" -print)
if [ ${#fuzzy_drawables[@]} -gt 0 ]; then
echo "Fuzzy drawable match(es) found:"
for d in "${fuzzy_drawables[@]}"; do printf " - %s\n" "$d"; done
else
echo "No drawable matches found."
fi
fi
echo
# Search for occurrences of 'personal_id_credential_' in res xml files (colors/backgrounds, etc.)
echo "Searching for usages of 'personal_id_credential_' in res xml files..."
mapfile -t credential_matches < <(find . -type f -path "*/res/*" -iname "*.xml" -exec grep -nH "personal_id_credential_" {} \; || true)
if [ ${#credential_matches[@]} -eq 0 ]; then
echo "No occurrences of 'personal_id_credential_' found in res xml files."
else
echo "Occurrences found:"
for m in "${credential_matches[@]}"; do printf " - %s\n" "$m"; done
fiLength of output: 145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Working directory: $(pwd)"
echo
# Collect strings.xml files (robust to shells without /dev/fd/process-substitution)
strings_tmp=$(mktemp)
find . -type f -path "*/res/values*/strings*.xml" -print0 > "$strings_tmp" || true
if [ ! -s "$strings_tmp" ]; then
echo "No strings.xml files found under res/values*"
else
echo "Found strings.xml files:"
xargs -0 -n1 < "$strings_tmp" | sed 's/^/ - /'
fi
echo
if [ -s "$strings_tmp" ]; then
echo "Checking for name=\"personalid_work_history_pending\" in each strings.xml..."
xargs -0 -n1 < "$strings_tmp" | while IFS= read -r f; do
if grep -q 'name="personalid_work_history_pending"' "$f"; then
echo "Present: $f"
else
echo "Missing : $f"
fi
done
total=$(xargs -0 -n1 < "$strings_tmp" | wc -l)
present_count=$(xargs -0 -n1 < "$strings_tmp" | xargs -I{} grep -nH 'name="personalid_work_history_pending"' {} 2>/dev/null | wc -l || true)
echo "Summary: total=$total present=$present_count missing=$((total - present_count))"
fi
echo
# Drawable checks
echo "Searching for drawable ic_personalid_work_history_pending_person.* under res/drawable* ..."
drawables=$(find . -type f -path "*/res/drawable*" -iname "ic_personalid_work_history_pending_person.*" -print || true)
if [ -n "$drawables" ]; then
echo "Exact drawable(s) found:"
printf "%s\n" "$drawables" | sed 's/^/ - /'
else
echo "No exact drawable match found. Fuzzy search for 'personalid_work_history_pending' in res/drawable* ..."
find . -type f -path "*/res/drawable*" -iname "*personalid_work_history_pending*" -print | sed 's/^/ - /' || echo "No fuzzy matches found."
fi
echo
# Search for 'personal_id_credential_' usages in res XML files
echo "Searching for 'personal_id_credential_' in res XML files..."
find . -type f -path "*/res/*" -iname "*.xml" -print0 | xargs -0 grep -nH "personal_id_credential_" || echo "No occurrences found."
echo
rm -f "$strings_tmp"Length of output: 3102
Add missing translations and confirm drawable; plan resource-naming cleanup
- missing key personalid_work_history_pending in: ./app/res/values-large/strings.xml, ./app/res/values-lt/strings.xml, ./app/res/values-no/strings.xml, ./app/res/values-small/strings.xml, ./app/res/values-sw720dp-land/strings.xml, ./app/res/values-xlarge/strings.xml, ./commcare-support-library/src/main/res/values/strings.xml — add translations or ensure a fallback.
- drawable present: ./app/res/drawable/ic_personalid_work_history_pending_person.xml.
- occurrences of legacy naming (personal_id_credential_*) found in: ./app/res/drawable/personal_id_credential_pending.xml, ./app/res/layout/item_earned_credential.xml, ./app/res/layout/item_pending_credential.xml, ./app/res/values/colors.xml — consider mirroring names to *_work_history for consistency later.
🤖 Prompt for AI Agents
In app/res/layout/item_pending_credential.xml lines 20-21, the layout references
the string key personalid_work_history_pending which is missing from multiple
localized string files; add that key with appropriate translations to
./app/res/values-large/strings.xml, ./app/res/values-lt/strings.xml,
./app/res/values-no/strings.xml, ./app/res/values-small/strings.xml,
./app/res/values-sw720dp-land/strings.xml, ./app/res/values-xlarge/strings.xml
and ./commcare-support-library/src/main/res/values/strings.xml (or ensure the
base ./app/res/values/strings.xml provides a fallback), verify the drawable
./app/res/drawable/ic_personalid_work_history_pending_person.xml exists and is
referenced correctly, and to prepare for a future resource-name cleanup update
or create mirrored names for legacy personal_id_credential_* resources
(./app/res/drawable/personal_id_credential_pending.xml,
./app/res/layout/item_earned_credential.xml,
./app/res/layout/item_pending_credential.xml, ./app/res/values/colors.xml) so
both naming schemes resolve until you perform a coordinated rename.
| private fun evalInstalledAppsWorkHistory(): List<PersonalIdWorkHistory> { | ||
| val previousSandbox = CommCareApp.currentSandbox | ||
| val records = MultipleAppsUtil.getUsableAppRecords() | ||
| return try { | ||
| getCredentialsFromAppRecords(records) | ||
| getWorkHistoryFromAppRecords(records) | ||
| } finally { | ||
| CommCareApp.currentSandbox = previousSandbox | ||
| } | ||
| } |
There was a problem hiding this comment.
Sandbox context not fully restored; potential resource leak
getWorkHistoryFromAppRecords calls setupSandbox() repeatedly but neither tears down those sandboxes nor restores ProviderUtils/DB handles. Reassigning CommCareApp.currentSandbox alone isn’t enough.
Apply both this fix and the per-record teardown below:
private fun evalInstalledAppsWorkHistory(): List<PersonalIdWorkHistory> {
- val previousSandbox = CommCareApp.currentSandbox
+ val previousSandbox = CommCareApp.currentSandbox
val records = MultipleAppsUtil.getUsableAppRecords()
- return try {
- getWorkHistoryFromAppRecords(records)
- } finally {
- CommCareApp.currentSandbox = previousSandbox
- }
+ return try {
+ getWorkHistoryFromAppRecords(records)
+ } finally {
+ // Close whichever sandbox is currently seated
+ CommCareApp.currentSandbox?.teardownSandbox()
+ // Restore prior sandbox context (ProviderUtils, DB, file roots)
+ previousSandbox?.setupSandbox()
+ }
}📝 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.
| private fun evalInstalledAppsWorkHistory(): List<PersonalIdWorkHistory> { | |
| val previousSandbox = CommCareApp.currentSandbox | |
| val records = MultipleAppsUtil.getUsableAppRecords() | |
| return try { | |
| getCredentialsFromAppRecords(records) | |
| getWorkHistoryFromAppRecords(records) | |
| } finally { | |
| CommCareApp.currentSandbox = previousSandbox | |
| } | |
| } | |
| private fun evalInstalledAppsWorkHistory(): List<PersonalIdWorkHistory> { | |
| val previousSandbox = CommCareApp.currentSandbox | |
| val records = MultipleAppsUtil.getUsableAppRecords() | |
| return try { | |
| getWorkHistoryFromAppRecords(records) | |
| } finally { | |
| // Close whichever sandbox is currently seated | |
| CommCareApp.currentSandbox?.teardownSandbox() | |
| // Restore prior sandbox context (ProviderUtils, DB, file roots) | |
| previousSandbox?.setupSandbox() | |
| } | |
| } |
| override fun onBindViewHolder(holder: WorkHistoryViewHolder, position: Int) { | ||
| val item = workHistoryList[position] | ||
| with(holder.binding) { | ||
| tvAppName.text = item.title | ||
| tvActivity.text = StringUtils.getLocalizedLevel(item.level, holder.itemView.context) | ||
| } | ||
| } |
There was a problem hiding this comment.
Wire up the click listener; it’s currently unused.
onBindViewHolder doesn’t invoke listener.onWorkHistoryItemClick, so items are not clickable.
Apply:
override fun onBindViewHolder(holder: WorkHistoryViewHolder, position: Int) {
val item = workHistoryList[position]
with(holder.binding) {
tvAppName.text = item.title
tvActivity.text = StringUtils.getLocalizedLevel(item.level, holder.itemView.context)
}
+ holder.binding.root.setOnClickListener {
+ val pos = holder.bindingAdapterPosition
+ if (pos != RecyclerView.NO_POSITION) {
+ listener.onWorkHistoryItemClick(workHistoryList[pos])
+ }
+ }
}📝 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.
| override fun onBindViewHolder(holder: WorkHistoryViewHolder, position: Int) { | |
| val item = workHistoryList[position] | |
| with(holder.binding) { | |
| tvAppName.text = item.title | |
| tvActivity.text = StringUtils.getLocalizedLevel(item.level, holder.itemView.context) | |
| } | |
| } | |
| override fun onBindViewHolder(holder: WorkHistoryViewHolder, position: Int) { | |
| val item = workHistoryList[position] | |
| with(holder.binding) { | |
| tvAppName.text = item.title | |
| tvActivity.text = StringUtils.getLocalizedLevel(item.level, holder.itemView.context) | |
| } | |
| holder.binding.root.setOnClickListener { | |
| val pos = holder.bindingAdapterPosition | |
| if (pos != RecyclerView.NO_POSITION) { | |
| listener.onWorkHistoryItemClick(workHistoryList[pos]) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/org/commcare/adapters/WorkHistoryPendingAdapter.kt around lines 28–34
the click listener is never invoked so list items aren’t clickable; wire it by
attaching a click listener to the item view (e.g., holder.itemView or
holder.binding.root) inside onBindViewHolder that first checks
holder.adapterPosition != RecyclerView.NO_POSITION and then calls the adapter’s
listener method (listener.onWorkHistoryItemClick) with the current item (or
position) so clicks are forwarded to the consumer.
| SqlStorage.dropTable(db, PersonalIdWorkHistory.STORAGE_KEY); | ||
| addTableForNewModel(db, PersonalIdWorkHistory.STORAGE_KEY, new PersonalIdWorkHistory()); | ||
| } finally { |
There was a problem hiding this comment.
🧩 Analysis chain
Verify assumption before dropping table (potential data loss).
Comment says the table hasn’t been populated yet. Given the rename-only nature of this PR, please confirm there are no writes to PersonalIdWorkHistory (or the same STORAGE_KEY) prior to v18; otherwise we’ll wipe user data on upgrade.
Run:
🏁 Script executed:
#!/bin/bash
# Find any storage usages and potential writes for PersonalIdWorkHistory
rg -nP --type=java --type=kt -C3 'SqlStorage<[^>]*PersonalIdWorkHistory|PersonalIdWorkHistory\.STORAGE_KEY'
# Show write() calls in files that reference PersonalIdWorkHistory
fd -t f -e java -e kt | xargs -I{} bash -lc 'rg -q "PersonalIdWorkHistory" "{}" && rg -n -C2 "\.write\s*\(" "{}"'Length of output: 99087
Do not drop PersonalIdWorkHistory — preserve/migrate existing data.
There are write calls that persist PersonalIdWorkHistory records (storage.write(...) and multiple newStorage.write(...) in the upgrader flow), so dropping the table on upgrade will erase user data; implement a migration/copy of existing records or rename the STORAGE_KEY instead.
Location: app/src/org/commcare/models/database/connect/ConnectDatabaseUpgrader.java.
🤖 Prompt for AI Agents
In app/src/org/commcare/models/database/connect/ConnectDatabaseUpgrader.java
around lines 642-644, the current upgrader drops PersonalIdWorkHistory which
will erase persisted user records; instead remove the SqlStorage.dropTable call
and implement a migration: open the existing SqlStorage under
PersonalIdWorkHistory.STORAGE_KEY, read all existing records into memory or
stream them, create the new/newer table (or new STORAGE_KEY) with
addTableForNewModel, and insert/copymap each existing record into the new
storage key using storage.write(...) (or update their keys if renaming). Ensure
you preserve all relevant fields, handle potential schema differences with
field-by-field mapping, wrap the migration in a transaction, update any
metadata/indices, and only delete the old table after successful copy (or leave
it intact and mark migrated) while handling exceptions to avoid data loss.
| builder = new TableBuilder(PersonalIdWorkHistory.class); | ||
| database.execSQL(builder.getTableCreateString()); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Table rename/migration risk: verify STORAGE_KEY continuity or bump DB version.
Creating the table from PersonalIdWorkHistory.class is fine, but if the previous model’s STORAGE_KEY differed (e.g., personal_id_credential), existing installs on version 18 will not run any migration and will hit “no such table” at runtime. Either:
- keep the STORAGE_KEY identical to the previous model, or
- bump DB to v19 and add a migration that renames/copies the old table to the new key.
Run to confirm the previous STORAGE_KEY/table name and upgrader coverage:
If a migration is needed, bump the version here:
- private static final int CONNECT_DB_VERSION = 18;
+ private static final int CONNECT_DB_VERSION = 19;🏁 Script executed:
#!/bin/bash
# 1) What was the old storage key for PersonalIdCredential?
rg -nP 'class\s+PersonalIdCredential\b|STORAGE_KEY\s*=\s*' -C3 --glob '!**/build/**'
# 2) Any hard-coded "personal_id_credential" or "credential" table usage?
rg -nP '"personal_id_credential"|"credential"' app/src -C2 --glob '!**/build/**'
# 3) Check ConnectDatabaseUpgrader for a v18->v19 path that handles table rename/copy.
rg -n 'ConnectDatabaseUpgrader|onUpgrade|upgrade\(' -C3 app/src --glob '!**/build/**'Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Working directory: $(pwd)"
echo "Git repo present: $( [ -d .git ] && echo yes || echo no )"
echo "Top-level entries:"
ls -la | sed -n '1,120p'
run_search() {
local pattern="$1"
local target="${2:-.}"
if command -v rg >/dev/null 2>&1; then
rg --hidden --no-ignore-vcs -nP "$pattern" -C3 "$target" || true
else
git grep -n --line-number -E "$pattern" || true
fi
}
echo
echo "=== Search 1: class PersonalIdCredential or STORAGE_KEY assignments ==="
run_search "class\s+PersonalIdCredential\b|STORAGE_KEY\s*=" .
echo
echo "=== Search 2: hard-coded table names (personal_id_credential / credential) ==="
run_search '"personal_id_credential"|"personalIdCredential"|"credential"' app/src || run_search '"personal_id_credential"|"personalIdCredential"|"credential"' .
echo
echo "=== Search 3: ConnectDatabaseUpgrader / onUpgrade / upgrade occurrences ==="
run_search "class\s+ConnectDatabaseUpgrader\b|onUpgrade\s*\(|\bupgrade\s*\(" app/src || run_search "class\s+ConnectDatabaseUpgrader\b|onUpgrade\s*\(|\bupgrade\s*\(" .
echo
echo "=== Search 4: CONNECT_DB_VERSION value ==="
run_search "CONNECT_DB_VERSION\s*=\s*\d+" app/src || run_search "CONNECT_DB_VERSION\s*=\s*\d+" .
echo
echo "=== Show DatabaseConnectOpenHelper.java lines ~120-160 if present ==="
if [ -f app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java ]; then
nl -ba app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java | sed -n '120,160p'
else
echo "File not found at expected path; searching for filename..."
run_search "DatabaseConnectOpenHelper" .
fi
echo
echo "=== Done ==="Length of output: 56911
Table rename/migration required — preserve STORAGE_KEY or bump DB version + add migration
- Evidence: PersonalIdWorkHistory.kt defines STORAGE_KEY = "credential" (app/src/org/commcare/android/database/connect/models/PersonalIdWorkHistory.kt:57). DatabaseConnectOpenHelper still uses CONNECT_DB_VERSION = 18 (app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java:63) and its changelog references V18 changes to the old personal_id_credential table; the helper creates the new table from PersonalIdWorkHistory (lines ~142–144).
- Action (choose one):
- Restore the previous STORAGE_KEY (personal_id_credential), or
- Bump CONNECT_DB_VERSION to 19 (private static final int CONNECT_DB_VERSION = 19) and add a v18→v19 migration in app/src/org/commcare/models/database/connect/ConnectDatabaseUpgrader.java that renames/copies the old personal_id_credential table → credential before creating the new table.
🤖 Prompt for AI Agents
In app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java
around lines 142–144, the helper is creating the new PersonalIdWorkHistory table
while the model defines STORAGE_KEY = "credential" but the previous DB used
"personal_id_credential", so you must preserve data compatibility: either
restore the STORAGE_KEY to "personal_id_credential" in
app/src/org/commcare/android/database/connect/models/PersonalIdWorkHistory.kt to
match existing DB schema, or bump CONNECT_DB_VERSION to 19 in
DatabaseConnectOpenHelper (private static final int CONNECT_DB_VERSION = 19) and
add a v18→v19 migration in
app/src/org/commcare/models/database/connect/ConnectDatabaseUpgrader.java that
renames or copies the old personal_id_credential table to credential before
creating the new table; pick one approach, implement the corresponding change,
and ensure the migration is covered by tests and preserves existing rows.
|
Bumping review on this as I am afraid it might cause a bunch of conflicts. |
|
@shubham1g5 This branch shows at more places credentials than work history viz. BaseDrawerActivity, activity_personal_id_credential.xml, fragment_work_history_earned.xml, colors.xml, ApiPersonalId.java, ApiService.java etc. Do you think we need to change them also? |
|
@Jignesh-dimagi corrected |
|
@Jignesh-dimagi bumping on review |
Product Description
Renames bunch of
Credentialreference to Work History on UI and in code.Safety Assurance
There are no functional changes in the PR.
Labels and Review