Implements Worker history #3221
Conversation
-Adapters and implementation of api -Designed view to share on whatsapp
jaypanchal-13
requested review from
Jignesh-dimagi,
OrangeAndGreen,
pm-dimagi and
shubham1g5
📝 WalkthroughWalkthroughThis change introduces a new feature for managing and displaying personal ID credentials within the app. It adds a new activity ( Sequence Diagram(s)sequenceDiagram
participant User
participant PersonalIdCredentialActivity
participant PersonalIdCredentialViewModel
participant API/DB
participant RecyclerViewAdapter
participant SelectedPersonalIdCredentialActivity
User->>PersonalIdCredentialActivity: Launch activity
PersonalIdCredentialActivity->>PersonalIdCredentialViewModel: retrieveCredentials()
PersonalIdCredentialViewModel->>API/DB: Fetch user and credentials
API/DB-->>PersonalIdCredentialViewModel: Return credential data or error
PersonalIdCredentialViewModel-->>PersonalIdCredentialActivity: Update LiveData (credentials or error)
PersonalIdCredentialActivity->>RecyclerViewAdapter: setData(earned/uneared credentials)
User->>RecyclerViewAdapter: Click "View Credential"
RecyclerViewAdapter->>PersonalIdCredentialActivity: onViewCredentialClick(credential)
PersonalIdCredentialActivity->>SelectedPersonalIdCredentialActivity: Launch with credential details
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (5)
app/res/drawable/bg_hq_cred.xml (1)
3-18: See previous comment – consolidation recommendedapp/res/drawable/ic_hq_share_bg.xml (2)
2-5: Same intrinsic-size concern as previous drawableSee earlier comment about 529 dp vectors. Applies here as well.
8-22: Literal colours duplicated across drawablesSame refactor suggestion as for
ic_delivery_share_bg.xml.app/res/drawable/ic_learn_share_bg.xml (2)
2-5: Oversized intrinsic width/heightDitto – remove or down-scale the 529 dp size.
8-22: Extract hard-coded colour literalsDitto – move to
colors.xml.
🧹 Nitpick comments (25)
app/res/values/styles.xml (1)
358-361: Declare an explicit parent and useshapeAppearanceattributes to avoid style-resolution surprises.
Leavingparentempty forces the style engine to fall back to the defaultThemewhich may not include Material attributes; on some devices this silently drops the round-corners settings.
Prefer:<style name="circleImageView" parent="Widget.MaterialComponents.ImageView.ShapeableImageView"> <item name="shapeAppearanceOverlay">@style/CircleShapeAppearance</item> </style> <style name="CircleShapeAppearance"> <item name="cornerFamily">rounded</item> <item name="cornerSize">50%</item> </style>This follows Material-Components conventions, keeps the attributes namespaced, and avoids polluting every consuming view with
app:shapeAppearanceOverlay.app/res/drawable/ic_arrow_forward.xml (1)
1-9: Inline white fill limits theming flexibility – reference a color resource instead.Hard-coding
#ffffffmeans any dark-mode / dynamic-theming pass has to duplicate the drawable.
Replace with:- android:fillColor="#ffffff"/> + android:fillColor="@color/white"/>Alternatively drop
fillColorand tint the ImageView at runtime.app/res/drawable/ic_cloud_sync.xml (1)
1-9: Same theming concern as the arrow icon – prefer a color resource or runtime tint.Using
@color/white(or removingfillColorand relying onandroid:tint) will keep the icon consistent with color-blind/dark themes without duplicating assets.app/res/drawable/bg_rounded_blue_70.xml (1)
3-16: Redundant stroke & extreme radius – lighten the drawable.
- Stroke and fill share the same color, so the 1 dp stroke is invisible; drop it or use a contrasting color.
- A fixed
70dpradius forces >140 dp wide/height views to stay fully rounded; if the host view is smaller this becomes an oval. ConsidershapeAppearanceOverlaywithcornerSize=50%for automatic circles.- <stroke - android:width="1dp" - android:color="@color/connect_delivery_check_circle" />app/res/drawable/bg_hq_cred_8.xml (2)
3-7: Stroke is visually redundant – border merges into fillBoth the
<stroke>and<solid>use the same@color/connect_message_large_icon_color. As a result the 1 dp border is indistinguishable from the fill and provides no visual affordance.- <stroke - android:width="1dp" - android:color="@color/connect_message_large_icon_color" /> - - <solid android:color="@color/connect_message_large_icon_color" /> + <!-- Keep a subtle border that is still visible --> + <stroke + android:width="1dp" + android:color="@color/connect_message_large_icon_color" /> + + <!-- Use 8 % opacity to differentiate the interior, or pick a + dedicated background colour in your palette --> + <solid android:color="@color/connect_message_large_icon_color_08" />If the intention is truly a fully-filled pill, consider deleting the
<stroke>block to avoid a dead attribute.
9-14: Avoid hard-coding 1 dp padding in drawableLet the consuming layout control padding so the same drawable can be re-used in contexts with different density buckets and spacing requirements.
app/res/drawable/bg_learn_cred.xml (1)
15-17: Only bottom corners are roundedIf the drawable will ever be used where the top edge is visible, the hard 90° corner will feel inconsistent with the other credential cards. Confirm usage context; otherwise add
topLeftRadius/topRightRadius.app/res/drawable/bg_rounded_blue_12.xml (2)
3-7: Border invisible – same colour as fillIdentical colour for
<stroke>and<solid>yields no perceptible border. Either drop the<stroke>entirely or pick a slightly darker / lighter tint for the stroke.
9-14: Remove intrinsic paddingPer team precedent (PR #3070 learning), shape drawables should not declare intrinsic padding; leave spacing to layouts so the background is purely a background.
app/res/values/strings.xml (1)
635-638: Smart quotes may trip the aapt compiler
no_credentials_currently_availableuses curly quotes“Refresh”. These often survive but can break when copied into other IDEs or localisation tools. Prefer straight quotes to avoid encoding issues.app/res/menu/menu_work_history.xml (1)
3-7: ConsiderifRoomrather thanalwaysfor overflow-safe toolbars
app:showAsAction="always"forces the icon even on narrow screens, risking menu overflow. Unless this is a critical action,ifRoom|collapseActionViewis safer.app/AndroidManifest.xml (1)
623-627: Explicitly pin UI-behavioural attributes for the new Activity
SelectedPersonalIdCredentialActivityis added withandroid:exported="false"(👍).
For consistency with the rest of the “connect” screens (PersonalIdActivityat l.169) consider also setting:android:screenOrientation="portrait" android:windowSoftInputMode="adjustResize"unless the design intentionally allows rotation / default input-mode. Being explicit prevents surprises after future target-Sdk bumps or OEM quirks.
app/res/drawable/ic_share.xml (1)
6-8: Hard-coded fillColor limits theming / dark-modeUsing a literal
#3A42C7impedes icon tinting. Prefer theme attributes so the icon adapts automatically:- android:fillColor="#3A42C7"/> + android:fillColor="?attr/colorOnPrimary"/>(or the appropriate semantic colour).
This keeps the vector drawable reusable across themes and brand palettes.app/res/drawable/ic_more_info.xml (1)
10-11: Same theming concern asic_share.xmlReplace the literal colour with a theme attribute to enable automatic tinting in dark-mode / white-label builds.
app/res/drawable/bg_delivery_cred.xml (1)
3-18: Duplicate shape parameters across credential backgrounds
bg_delivery_cred.xml,bg_hq_cred.xml,bg_learn_cred.xml, … all repeat the same<stroke>,<padding>and corner radius, differing only by colour.
Extract the common shape into a single drawable (e.g.base_cred_bg.xml) and reuse it via a<item android:drawable="@drawable/base_cred_bg" android:tint="@color/…"/>layer-list or programmatic tint. This removes duplication and eases later visual tweaks.app/res/drawable/ic_available_not_earned.xml (2)
2-5: Avoid hard-coding an intrinsic size of 164 dp in the vector headerDeclaring
android:width|height="164dp"bakes a large bitmap size into every place this drawable is used, increasing memory usage and making it harder for layouts to scale the asset. Let the containingImageViewdecide the rendered size and keep only the viewport attributes here.- android:width="164dp" - android:height="164dp" + android:width="24dp" + android:height="24dp"(or simply remove them and set size in the layout).
9-24: Move hard-coded colours intocolors.xmlfor themeabilityColours like
#D9D9D9,#343A40,#3A42C7,#F8F9FAare repeated literal values.
Expose them as named resources (@color/…) so that:
- Dark-/high-contrast themes can override them.
- Designers can update the palette from a single place.
app/res/drawable/ic_delivery_share_bg.xml (2)
2-5: 529 dp vectors are oversized – consider removing the intrinsic sizeA 529×485 dp intrinsic size will allocate very large canvases on high-dpi devices (>4 K px). That is unnecessary for a decorative background and can hurt memory/GPU throughput. Rely on the viewport only or down-scale to a modest default (e.g. 24 dp).
8-22: Centralise stroke / fill coloursThe same orange
#FC5F36, blue#3843D0and white occur in multiple new share-background vectors. Extract them tocolors.xmlso they can be reused and, if required, tinted at runtime.app/res/layout/item_personal_id_credential.xml (2)
14-22: Rename generic idviewto something meaningful
@+id/viewgives no clue in Kotlin view-binding or in Constraint references. A descriptive id (e.g.vHeaderStrip) improves readability and maintainability.
142-156: Fixed 100 dp avatar may not scale on small devicesConsider using a dimension resource (e.g.
@dimen/avatar_medium) and letting designers tune per-density values. Hard-coding pixel-equivalent dp hampers responsiveness.app/src/org/commcare/adapters/PersonalIdCredentialAdapter.kt (1)
43-47: Consider using DiffUtil for better performance.While
notifyDataSetChanged()works correctly, usingDiffUtilwould provide better performance by only updating changed items instead of refreshing the entire list.fun setData(newList: List<PersonalIdCredential>) { - credentialList.clear() - credentialList.addAll(newList) - notifyDataSetChanged() + val diffCallback = PersonalIdCredentialDiffCallback(credentialList, newList) + val diffResult = DiffUtil.calculateDiff(diffCallback) + credentialList.clear() + credentialList.addAll(newList) + diffResult.dispatchUpdatesTo(this) }You would also need to implement a
DiffCallbackclass to compare credential items.app/src/org/commcare/activities/connect/SelectedPersonalIdCredentialActivity.kt (1)
7-7: Remove unused import.The
ActivityPersonalIdCredentialBindingimport is not used in this file.-import org.commcare.dalvik.databinding.ActivityPersonalIdCredentialBindingapp/res/layout/credential_share_view.xml (1)
109-135: Consider simplifying the guideline structure.The layout uses many guidelines (10%, 25%, 75%, 90%) which may be over-engineered. Consider whether some of these can be replaced with margins or simplified constraint relationships while maintaining the same visual result.
For example, the badge text elements could use consistent horizontal margins instead of multiple guidelines:
- app:layout_constraintEnd_toEndOf="@id/glEnd25" - app:layout_constraintStart_toStartOf="@id/glStart25" + android:layout_marginHorizontal="24dp" + app:layout_constraintEnd_toEndOf="parent" + app:layout_constraintStart_toStartOf="parent"app/res/layout/activity_selected_personal_id_credential.xml (1)
169-176: Remove unnecessary invisible View elementThe invisible View element appears to be used for layout spacing but adds unnecessary complexity. Consider removing it and adjusting the tvShare constraints directly.
<androidx.appcompat.widget.AppCompatTextView android:id="@+id/tvShare" - android:layout_width="wrap_content" + android:layout_width="0dp" android:layout_height="wrap_content" android:layout_marginTop="30dp" android:drawableEnd="@drawable/ic_share" android:fontFamily="@font/nunito_sans" android:text="@string/share_your_accomplishment_with_friends_and_family" android:textColor="@color/connect_blue_color" android:textSize="13sp" android:drawablePadding="2dp" app:layout_constraintStart_toStartOf="@id/guideStart5" + app:layout_constraintEnd_toEndOf="@id/guideEnd5" app:layout_constraintTop_toBottomOf="@id/clInfo" /> - <View - android:layout_width="1dp" - android:layout_height="0dp" - app:layout_constraintEnd_toEndOf="@id/guideEnd5" - app:layout_constraintStart_toEndOf="@id/tvShare" - app:layout_constraintTop_toTopOf="@id/tvShare" - app:layout_constraintBottom_toBottomOf="@id/tvShare"/>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
app/res/drawable/ic_dummy_profile.jpgis excluded by!**/*.jpgapp/res/font/nunito_sans.ttfis excluded by!**/*.ttfapp/res/font/work_sans.ttfis excluded by!**/*.ttf
📒 Files selected for processing (27)
app/AndroidManifest.xml(1 hunks)app/res/drawable/bg_delivery_cred.xml(1 hunks)app/res/drawable/bg_hq_cred.xml(1 hunks)app/res/drawable/bg_hq_cred_8.xml(1 hunks)app/res/drawable/bg_learn_cred.xml(1 hunks)app/res/drawable/bg_rounded_blue_12.xml(1 hunks)app/res/drawable/bg_rounded_blue_70.xml(1 hunks)app/res/drawable/ic_arrow_forward.xml(1 hunks)app/res/drawable/ic_available_not_earned.xml(1 hunks)app/res/drawable/ic_cloud_sync.xml(1 hunks)app/res/drawable/ic_delivery_share_bg.xml(1 hunks)app/res/drawable/ic_hq_share_bg.xml(1 hunks)app/res/drawable/ic_learn_share_bg.xml(1 hunks)app/res/drawable/ic_more_info.xml(1 hunks)app/res/drawable/ic_share.xml(1 hunks)app/res/layout/activity_personal_id_credential.xml(1 hunks)app/res/layout/activity_selected_personal_id_credential.xml(1 hunks)app/res/layout/credential_share_view.xml(1 hunks)app/res/layout/item_personal_id_credential.xml(1 hunks)app/res/menu/menu_work_history.xml(1 hunks)app/res/values/colors.xml(1 hunks)app/res/values/strings.xml(1 hunks)app/res/values/styles.xml(1 hunks)app/src/org/commcare/activities/connect/PersonalIdCredentialActivity.kt(1 hunks)app/src/org/commcare/activities/connect/SelectedPersonalIdCredentialActivity.kt(1 hunks)app/src/org/commcare/activities/connect/viewmodel/PersonalIdCredentialViewModel.kt(1 hunks)app/src/org/commcare/adapters/PersonalIdCredentialAdapter.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (26)
📓 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.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#0
File: :0-0
Timestamp: 2025-05-08T13:40:19.645Z
Learning: PR #3048 introduces a comprehensive messaging system in the Connect feature, implementing secure encryption using AES-GCM for message content, proper channel management with consent flows, and a well-designed UI separation between sent and received messages with real-time notification integration.
app/res/drawable/ic_arrow_forward.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
app/res/drawable/bg_delivery_cred.xml (1)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
app/res/drawable/bg_rounded_blue_12.xml (1)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
app/AndroidManifest.xml (3)
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.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3037
File: app/src/org/commcare/connect/ConnectConstants.java:11-15
Timestamp: 2025-04-21T18:48:08.330Z
Learning: Request codes used for startActivityForResult should be unique throughout the application, even if they're used in different activities. COMMCARE_SETUP_CONNECT_LAUNCH_REQUEST_CODE and STANDARD_HOME_CONNECT_LAUNCH_REQUEST_CODE should have different values.
app/res/drawable/ic_share.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
app/res/values/styles.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/bg_hq_cred_8.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
app/res/drawable/ic_cloud_sync.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
app/res/drawable/ic_more_info.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/bg_hq_cred.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
app/res/drawable/bg_rounded_blue_70.xml (1)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
app/res/drawable/bg_learn_cred.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
app/res/drawable/ic_available_not_earned.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
app/res/layout/activity_personal_id_credential.xml (1)
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.
app/res/values/colors.xml (1)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
app/res/drawable/ic_hq_share_bg.xml (4)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
Learnt from: shubham1g5
PR: dimagi/commcare-android#2935
File: app/src/org/commcare/gis/EntityMapActivity.java:134-142
Timestamp: 2025-01-13T11:29:50.055Z
Learning: Google Maps SDK internally manages bitmap lifecycle for marker icons created through BitmapDescriptorFactory, so manual bitmap recycling is not needed in EntityMapActivity.java.
app/res/layout/item_personal_id_credential.xml (1)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3093
File: app/res/layout/dialog_payment_confirmation.xml:14-17
Timestamp: 2025-05-09T07:00:45.949Z
Learning: Ignore false positives about unsupported shadow attributes (android:shadowColor, shadowDx, shadowDy, and shadowRadius) in CardView elements when they don't actually exist in the code.
app/src/org/commcare/activities/connect/viewmodel/PersonalIdCredentialViewModel.kt (3)
Learnt from: shubham1g5
PR: dimagi/commcare-android#3042
File: app/src/org/commcare/fragments/BreadcrumbBarViewModel.java:50-55
Timestamp: 2025-04-21T15:02:17.492Z
Learning: ViewModels should not store View or Activity references as this can cause memory leaks. Unlike Fragments with setRetainInstance(true), ViewModels don't have automatic view detachment mechanisms. When migrating from Fragments to ViewModels, view references should be replaced with data-only state in the ViewModel.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3108
File: app/src/org/commcare/models/connect/ConnectLoginJobListModel.java:79-92
Timestamp: 2025-06-20T15:51:14.157Z
Learning: The ConnectLoginJobListModel class in app/src/org/commcare/models/connect/ConnectLoginJobListModel.java does not need to implement Parcelable interface as it is not passed between Android activities or fragments.
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.
app/src/org/commcare/activities/connect/SelectedPersonalIdCredentialActivity.kt (6)
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.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3037
File: app/src/org/commcare/connect/ConnectIDManager.java:233-243
Timestamp: 2025-04-22T15:48:29.346Z
Learning: Never instantiate Android Activity classes directly with 'new'. Activities should only be created through the Android framework using Intents.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3037
File: app/src/org/commcare/connect/ConnectIDManager.java:233-243
Timestamp: 2025-04-22T15:48:29.346Z
Learning: Never instantiate Android Activity classes directly with 'new' as it bypasses the Android component lifecycle. Activities should only be created by the system through Intents. Non-static instance fields don't need manual resetting as they'll be initialized with their default values when a new activity instance is properly created.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/src/org/commcare/fragments/SelectInstallModeFragment.java:201-205
Timestamp: 2025-05-22T14:26:41.341Z
Learning: In SelectInstallModeFragment.java, the showConnectErrorMessage method intentionally omits null checks because it's called at a specific point in the startup flow where UI is guaranteed to be loaded. It's designed to crash if activity or view is null to make potential issues immediately visible rather than hiding them with defensive programming.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3093
File: app/res/navigation/nav_graph_connect_messaging.xml:41-45
Timestamp: 2025-05-09T10:57:41.073Z
Learning: In the CommCare Android codebase, the navigation graph for Connect messaging (`nav_graph_connect_messaging.xml`) intentionally uses `channel_id` as the argument name in the connectMessageFragment, despite using `channelId` in other parts of the same navigation graph. This naming difference is by design in the refactored code.
app/res/layout/credential_share_view.xml (1)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
app/res/drawable/ic_delivery_share_bg.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
app/res/layout/activity_selected_personal_id_credential.xml (2)
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.
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.
app/res/values/strings.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/strings.xml:73-73
Timestamp: 2025-05-07T06:50:52.518Z
Learning: The empty `remote_form_payload_url` string resource in strings.xml is intentional legacy code and should be preserved as-is.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/res/values-ti/strings.xml:350-350
Timestamp: 2025-05-22T14:32:53.133Z
Learning: PersonalID and Connect features haven't been translated to Spanish, Lithuanian, or Norwegian yet, so users with those language settings see the English strings by default.
app/res/drawable/ic_learn_share_bg.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
app/src/org/commcare/activities/connect/PersonalIdCredentialActivity.kt (6)
Learnt from: shubham1g5
PR: dimagi/commcare-android#3042
File: app/src/org/commcare/fragments/BreadcrumbBarViewModel.java:50-55
Timestamp: 2025-04-21T15:02:17.492Z
Learning: ViewModels should not store View or Activity references as this can cause memory leaks. Unlike Fragments with setRetainInstance(true), ViewModels don't have automatic view detachment mechanisms. When migrating from Fragments to ViewModels, view references should be replaced with data-only state in the ViewModel.
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.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3093
File: app/res/navigation/nav_graph_connect_messaging.xml:41-45
Timestamp: 2025-05-09T10:57:41.073Z
Learning: In the CommCare Android codebase, the navigation graph for Connect messaging (`nav_graph_connect_messaging.xml`) intentionally uses `channel_id` as the argument name in the connectMessageFragment, despite using `channelId` in other parts of the same navigation graph. This naming difference is by design in the refactored code.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3108
File: app/src/org/commcare/models/connect/ConnectLoginJobListModel.java:79-92
Timestamp: 2025-06-20T15:51:14.157Z
Learning: The ConnectLoginJobListModel class in app/src/org/commcare/models/connect/ConnectLoginJobListModel.java does not need to implement Parcelable interface as it is not passed between Android activities or fragments.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/src/org/commcare/fragments/SelectInstallModeFragment.java:201-205
Timestamp: 2025-05-22T14:26:41.341Z
Learning: In SelectInstallModeFragment.java, the showConnectErrorMessage method intentionally omits null checks because it's called at a specific point in the startup flow where UI is guaranteed to be loaded. It's designed to crash if activity or view is null to make potential issues immediately visible rather than hiding them with defensive programming.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (11)
app/res/values/colors.xml (1)
217-224: Good: new palette entries are unique and descriptive.No conflicts detected with existing names; consistent snake_case naming matches the rest of the file.
app/res/drawable/bg_learn_cred.xml (1)
3-5: 3 dp stroke stands out – confirm with designNeighbouring credential drawables (
bg_hq_cred.xml,bg_delivery_cred.xml) use a 1 dp stroke. Using 3 dp here will make this tile visually heavier. Double-check with the design spec for consistency.app/res/layout/item_personal_id_credential.xml (1)
118-133:drawableEndwithout RTL fallback may mis-render pre-API 17If minSdk < 17,
drawableEndis ignored. Make it explicit:android:drawableStart="@drawable/ic_arrow_forward" android:drawableEnd="@drawable/ic_arrow_forward"or switch to
material:iconEnd.app/src/org/commcare/adapters/PersonalIdCredentialAdapter.kt (1)
1-52: LGTM! Well-implemented RecyclerView adapter with proper patterns.The adapter follows Android best practices with ViewBinding, proper click handling through interfaces, and clean separation of concerns.
app/src/org/commcare/activities/connect/SelectedPersonalIdCredentialActivity.kt (1)
10-27: Good implementation of activity setup and navigation.The ViewBinding usage, action bar configuration, and defensive programming with
isFinishingcheck are well implemented.app/src/org/commcare/activities/connect/viewmodel/PersonalIdCredentialViewModel.kt (2)
12-18: Excellent LiveData implementation following MVVM patterns.The private/public LiveData pattern with proper exposure is correctly implemented, providing reactive data updates to the UI layer.
20-35: No context retention detected — safe to use
Both ConnectUserDatabaseUtil.getUser()/storeUser() and PersonalIdApiHandler.retrieveCredentials() only use the passed‐in Context within their method bodies and do not store it in any fields. The ViewModel does not hold onto the Context reference after the call returns, so there’s no risk of a memory leak here.app/res/layout/activity_personal_id_credential.xml (1)
1-121: Excellent layout structure with proper constraint management.The layout demonstrates good Android UI practices:
- Proper use of NestedScrollView for scrollable content
- Well-organized sections with clear visual hierarchy
- Effective use of barriers for layout management
- Consistent styling and spacing throughout
- Proper constraint relationships preventing layout issues
The separation of earned and yet-to-be-earned credentials provides clear UX distinction.
app/res/layout/credential_share_view.xml (1)
1-154: Well-structured credential sharing layout with good visual hierarchy.The layout effectively presents credential information with proper visual separation and styling. The use of ShapeableImageView for circular profile images and consistent text styling creates a professional appearance.
app/res/layout/activity_selected_personal_id_credential.xml (1)
25-25: Replace placeholder drawable with proper default iconThe
ic_dummy_profileappears to be a placeholder. Consider using a proper default profile icon or ensure this is replaced with actual profile images when the activity loads.app/src/org/commcare/activities/connect/PersonalIdCredentialActivity.kt (1)
112-115: Clarify the commented cloud sync functionalityThe cloud sync menu action has its implementation commented out. Consider:
- Removing the menu item if the feature is not ready
- Adding a TODO comment explaining when this will be implemented
- Showing a "Coming soon" message to users
app/src/org/commcare/activities/connect/SelectedPersonalIdCredentialActivity.kt
Outdated
Show resolved
Hide resolved
app/src/org/commcare/activities/connect/PersonalIdCredentialActivity.kt
Outdated
Show resolved
Hide resolved
app/src/org/commcare/activities/connect/PersonalIdCredentialActivity.kt
Outdated
Show resolved
Hide resolved
| private fun updateCredentialLists(data: List<PersonalIdCredential>) { | ||
| // filter data for earned credential and yetToBeEarned credential | ||
| earnedCredentialAdapter.setData(data) | ||
| yetToBeEarnedCredentialsAdapter.setData(data) | ||
| } |
There was a problem hiding this comment.
Implement filtering logic for earned vs yet-to-be-earned credentials
The method comment indicates filtering should occur, but the implementation sets the same data to both adapters:
private fun updateCredentialLists(data: List<PersonalIdCredential>) {
- // filter data for earned credential and yetToBeEarned credential
- earnedCredentialAdapter.setData(data)
- yetToBeEarnedCredentialsAdapter.setData(data)
+ val earnedCredentials = data.filter { it.isEarned }
+ val yetToBeEarnedCredentials = data.filter { !it.isEarned }
+
+ earnedCredentialAdapter.setData(earnedCredentials)
+ yetToBeEarnedCredentialsAdapter.setData(yetToBeEarnedCredentials)
+
+ // Update counts in UI
+ binding.tvEarnedCredentials.text =
+ getString(R.string.earned_credentials, earnedCredentials.size)
+ binding.tvCredentialsYetToBeEarned.text =
+ getString(R.string.credentials_yet_to_be_earned, yetToBeEarnedCredentials.size)
}Note: This assumes PersonalIdCredential has an isEarned property. Adjust the filtering logic based on your actual data model.
🤖 Prompt for AI Agents
In app/src/org/commcare/activities/connect/PersonalIdCredentialActivity.kt
around lines 88 to 92, the updateCredentialLists method currently sets the same
full data list to both earnedCredentialAdapter and
yetToBeEarnedCredentialsAdapter without filtering. To fix this, filter the input
data list into two separate lists based on the isEarned property of
PersonalIdCredential: one list for earned credentials and another for
yet-to-be-earned credentials, then set these filtered lists to the respective
adapters.
app/src/org/commcare/activities/connect/PersonalIdCredentialActivity.kt
Outdated
Show resolved
Hide resolved
shubham1g5
changed the title
There was a problem hiding this comment.
Do we need this file? Not sure its good idea to show in default.
There was a problem hiding this comment.
@Jignesh-dimagi As of now it is used to show icon will remove it once finalized from were this pic will get
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toBottomOf="@id/barrierEarnedCredentials" /> | ||
|
|
||
| <androidx.recyclerview.widget.RecyclerView |
There was a problem hiding this comment.
@jaypanchal-13 The design shows that it has expandable recycler view meaning that when click on title, recycler view open and close.
There was a problem hiding this comment.
@Jignesh-dimagi Am I missing something?. I think there is no description for closing this recyclerview
There was a problem hiding this comment.
@jaypanchal-13 I mean in Figma, screens shows 2 titles for earned and yet to earn title resp and it looks like that whenever user clicks on either of these titles, it will show the relevant credentials in recyclerview
There was a problem hiding this comment.
@Jignesh-dimagi Yes I also presumed that but in mockup description there is no point regarding closing view
There was a problem hiding this comment.
@jaypanchal-13 Please get it confirmed from someone, may be from Sankalp.
There was a problem hiding this comment.
@Jignesh-dimagi Yes Added comments in figma for it
There was a problem hiding this comment.
@Jignesh-dimagi I got confirmation from Sankalp on Figma that should not explandable list
| binding.tvEarnedCredentials.text = | ||
| getString(R.string.earned_credentials, 2) | ||
| binding.tvCredentialsYetToBeEarned.text = | ||
| getString(R.string.credentials_yet_to_be_earned, 0) |
There was a problem hiding this comment.
@jaypanchal-13 Seems like the number of credentials are hardcoded, shouldn't it come from API?
There was a problem hiding this comment.
@jaypanchal-13 Yes just for testing pupose hardcoded it
| personalIdCredentialViewModel.apiError.observe(this) { (code, throwable) -> | ||
| Logger.log( | ||
| "CREDENTIALS_API_ERROR", | ||
| "Code: $code, Throwable: ${throwable?.localizedMessage ?: "null"}" | ||
| ) |
There was a problem hiding this comment.
@jaypanchal-13 It should some UI/Toast to show that fetching credentials have been failed
There was a problem hiding this comment.
@Jignesh-dimagi I don't think we should show error code and error msg accept internet connectivity issue?
what do you think @shubham1g5?
There was a problem hiding this comment.
@jaypanchal-13 If there is error in retrieving the credentials, I think we should show it either in toast or some text view
There was a problem hiding this comment.
@jaypanchal-13 Okay will update it without showing error code just error msg
There was a problem hiding this comment.
@jaypanchal-13 this needs to call PersonalIdApiErrorHandler.handle(requireActivity(), failureCode, t) to get the right error message to display.
| } | ||
|
|
||
| R.id.cloud_sync -> { | ||
| // callCredentialApi() |
There was a problem hiding this comment.
Sync should reload the credentials?
There was a problem hiding this comment.
@Jignesh-dimagi Yes it will call an api with updated credentials. As of now commented it beacause credentials work is pending from backend
There was a problem hiding this comment.
think fine to write code here assuming that the API is working and we can test it later on.
| private fun setUiData() { | ||
| supportActionBar?.apply { | ||
| title = getString(R.string.my_earned_credential) | ||
| setDisplayShowHomeEnabled(true) | ||
| setDisplayHomeAsUpEnabled(true) | ||
| } | ||
| } |
There was a problem hiding this comment.
This function should set UI for displaying the credentials detail view.
There was a problem hiding this comment.
@Jignesh-dimagi Yes this func is for that. did not get you here?
There was a problem hiding this comment.
@jaypanchal-13 This activity doesn't have implementation for showing the credential details
There was a problem hiding this comment.
@Jignesh-dimagi Yes that data will be passed from previous screen(PersonalIdCredentualActivity) on clicking rv item. As of now there is no data so did not set here
| override fun onOptionsItemSelected(item: MenuItem): Boolean { | ||
| return when (item.itemId) { | ||
| android.R.id.home -> { | ||
| if (!isFinishing) { | ||
| this.onBackPressed() | ||
| } |
There was a problem hiding this comment.
Do we need this override?
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toBottomOf="@id/barrierEarnedCredentials" /> | ||
|
|
||
| <androidx.recyclerview.widget.RecyclerView |
There was a problem hiding this comment.
@jaypanchal-13 I mean in Figma, screens shows 2 titles for earned and yet to earn title resp and it looks like that whenever user clicks on either of these titles, it will show the relevant credentials in recyclerview
| private fun setUiData() { | ||
| supportActionBar?.apply { | ||
| title = getString(R.string.my_earned_credential) | ||
| setDisplayShowHomeEnabled(true) | ||
| setDisplayHomeAsUpEnabled(true) | ||
| } | ||
| } |
There was a problem hiding this comment.
@jaypanchal-13 This activity doesn't have implementation for showing the credential details
| startActivity( | ||
| Intent( | ||
| this@PersonalIdCredentialActivity, | ||
| SelectedPersonalIdCredentialActivity::class.java | ||
| ) |
There was a problem hiding this comment.
@jaypanchal-13 Start activity have intent with PersonalIdCredential as app needs to show the details in next screen?
There was a problem hiding this comment.
@Jignesh-dimagi This is item click of PersonalIdCredentialActivity on clicking it. SelectedPersonalIdCredentialActivity will open with clicked item data
…340-credential-list-feature
-Added code for sharing credentials on whatsapp
-Added code for sharing credentials on whatsapp
…l-list-feature # Conflicts: # app/res/values/strings.xml
- Converted class to kotlin
There was a problem hiding this comment.
remove this, we don't want to show a dummy photo in the actual app.
There was a problem hiding this comment.
@shubham1g5 I think we wil not get this image from credentials api. Do you know from where we will get image?
There was a problem hiding this comment.
this is the user image stored as photo in ConnectUserRecord
| @@ -0,0 +1,22 @@ | |||
| <vector xmlns:android="http://schemas.android.com/apk/res/android" | |||
There was a problem hiding this comment.
this file seems quite big for me for quite a simple vector. how are you producing these files ? we also already have a share icon if you can reuse that here - https://github.com/dimagi/commcare-android/blob/master/app/res/drawable/ic_share_24dp.xml
There was a problem hiding this comment.
@shubham1g5 We cannot use view which you are suggesting because ic_delivery_share_bg view is totally different and not a normal share image
There was a problem hiding this comment.
How are they different ? Is this not the share icon shown in first screenshot ?
There was a problem hiding this comment.
@shubham1g5 No it is bg image for image to be shared on whatsapp shown in last screenshot
There was a problem hiding this comment.
can we rename this aptly, ic_ should only be used for icons. how does credential_share_bg sound to you ?
There was a problem hiding this comment.
Removed this icon now no need of it as per new design
| android:width="529dp" | ||
| android:height="485dp" | ||
| android:viewportWidth="529" | ||
| android:viewportHeight="485"> |
There was a problem hiding this comment.
where are these values coming from ?
There was a problem hiding this comment.
@shubham1g5 If we download svg image from figma design these value get by default. Do you want me to change it or someting else?
There was a problem hiding this comment.
these should follow material design guidelines for icons. I don't think they are correctly set in figma files. In general all of these should be set to 24 for small icons or depending on where you are using them.
There was a problem hiding this comment.
I see the values as 529 x 485 here.
There was a problem hiding this comment.
Removed this icon now no need of it as per new design
| onReady(binding.root) | ||
| } | ||
|
|
||
| override fun onLoadCleared(placeholder: Drawable?) { |
There was a problem hiding this comment.
should we do binding.ivCredentialItem.setImageDrawable(null) as a clean up to remove any view references from Glide that might cause a mem leak ?
| LEARN("LEARN"), | ||
| DELIVER("DELIVER"), | ||
| APP_ACTIVITY("APP_ACTIVITY"), | ||
| UNKNOWN("UNKNOWN"); |
There was a problem hiding this comment.
there can't be an unknown credential type and we should just crash the app if that's the case.
| import org.commcare.android.nsd.MicroNode; | ||
| import org.commcare.android.nsd.NSDDiscoveryTools; | ||
| import org.commcare.android.nsd.NsdServiceListener; | ||
| import org.commcare.connect.PersonalIdManager; |
There was a problem hiding this comment.
changes in this class are unrelated to the PR and also not per out style guidelines. Make sure you correctly set your code style file as per readme for this project and we should remove these changes from here.
There was a problem hiding this comment.
changes in this class are unrelated to the PR and also not per out style guidelines. Make sure you correctly set your code style file as per readme for this project and we should remove these changes from here.
@shubham1g5 I think this import was removed while cleaning this fragment by me.
There was a problem hiding this comment.
the comment here is for all changes in the class and not just the imports. we should revert these changes if they are not related to the PR and done by mistake.
There was a problem hiding this comment.
Reverted it no change in this file
There was a problem hiding this comment.
I am still seeing changes in this file, are you not ?
| enum class CredentialType(val typeName: String) { | ||
| LEARN("LEARN"), | ||
| DELIVER("DELIVER"), | ||
| APP_ACTIVITY("APP_ACTIVITY"), |
There was a problem hiding this comment.
@shubham1g5 To change bg as per credential type these enum is created
There was a problem hiding this comment.
where are you getting this list from ?
There was a problem hiding this comment.
@shubham1g5 Once we will be able to test credentials api at that time we will need to manage bg color and accordingly for DELIVER,LEAR,HQ
app/src/org/commcare/android/database/connect/models/PersonalIdCredential.kt
Show resolved
Hide resolved
…340-credential-list-feature
…340-credential-list-feature
- Added filteration and sorting logic
| } | ||
| } | ||
|
|
||
| if (!corrupt.isNullOrEmpty()) { |
There was a problem hiding this comment.
Remove function corruptCredentialFromJson and directly crash the application with Json Exception received.
…l-list-feature # Conflicts: # app/res/values-es/strings.xml # app/res/values-fr/strings.xml # app/res/values-hi/strings.xml # app/res/values-pt/strings.xml # app/res/values-sw/strings.xml # app/res/values-ti/strings.xml # app/res/values/strings.xml # app/res/values/styles.xml # app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java
| @Persisting(9) | ||
| @MetaField(META_ISSUER_ENVIRONMENT) | ||
| var issuerEnvironment: String = "" | ||
|
|
||
| @Persisting(10) | ||
| @MetaField(META_SLUG) | ||
| var slug: String = "" |
There was a problem hiding this comment.
adding any new fields to the DB needs a DB migration, current the app crashes on this branch on updating from an older version.
| data class PersonalIdValidAndCorruptCredential( | ||
| val validCredentials: List<PersonalIdCredential>, | ||
| ) |
There was a problem hiding this comment.
do we still need this class now that we have removed corrupt credentials ?
| import java.util.TimeZone | ||
|
|
||
| @Throws(ParseException::class) | ||
| fun convertIsoDate(inputDate: String, outputPattern: String): String { |
There was a problem hiding this comment.
there is already a similar class now in code called ConnectDateUtils, can you add these methods over there only ?
| personalIdCredentialViewModel = ViewModelProvider( | ||
| this, ViewModelProvider.AndroidViewModelFactory.getInstance(application) | ||
| )[PersonalIdCredentialViewModel::class.java] |
There was a problem hiding this comment.
@jaypanchal-13 I think it requires simple solution provider like
personalIdCredentialViewModel = ViewModelProvider(this).get(PersonalIdCredentialViewModel::class.java);
There was a problem hiding this comment.
@Jignesh-dimagi I am using AndroidViewModel and passed context as an arguments for viewmodel. So for this case we should not intialize viewmodel here as you are suggesting
| if (corruptCredential.isNotEmpty()) { | ||
| val errorMessage = corruptCredential.joinToString( | ||
| separator = "\n\n", | ||
| prefix = "Found ${corruptCredential.size} corrupt credentials:\n" | ||
| ) | ||
| Logger.log("CorruptCredentials", errorMessage) | ||
| throw RuntimeException(errorMessage) |
There was a problem hiding this comment.
@jaypanchal-13 This will not give any idea what is corrupt. RuntimeException should crash with the JSON exception received, so that Firebase has proper exception logging.
| earnedCredentialAdapter = EarnedCredentialAdapter(listener = object : | ||
| EarnedCredentialAdapter.OnCredentialClickListener { | ||
| override fun onViewCredentialClick(credential: PersonalIdCredential) { | ||
|
|
There was a problem hiding this comment.
@jaypanchal-13 Did we get the clarity here what should happen whenever credential is clicked.
| fun retrieveAndProcessCredentials() { | ||
| object : PersonalIdApiHandler<List<PersonalIdCredential>>() { | ||
| override fun onSuccess(result: List<PersonalIdCredential>) { | ||
| val earned = result | ||
| val earnedAppIds = earned.map { it.appId }.toSet() | ||
| val installedApps = _installedAppRecords.value.orEmpty() | ||
|
|
||
| val pending = installedApps.filter { it.appId !in earnedAppIds } | ||
|
|
||
| _earnedCredentials.postValue( | ||
| earned.sortedByDescending { parseIsoDateForSorting(it.issuedDate) } | ||
| ) | ||
| _pendingCredentials.postValue( | ||
| pending.sortedByDescending { parseIsoDateForSorting(it.issuedDate) } | ||
| ) | ||
| } | ||
|
|
||
|
|
||
| override fun onFailure( | ||
| failureCode: PersonalIdOrConnectApiErrorCodes, | ||
| t: Throwable? | ||
| ) { | ||
| _apiError.postValue(failureCode to t) | ||
| } |
There was a problem hiding this comment.
@jaypanchal-13 Please put in viewModelScope to get it canceled whenever activity is destroyed while call in progress
…l-list-feature # Conflicts: # app/src/org/commcare/models/database/connect/ConnectDatabaseUpgrader.java
|
@jaypanchal-13 @Jignesh-dimagi the DB migration for credentials table was handled incorreclty here, we can't really change code for older versions migration as that is something that has already happened for apps out there. I have corrected it in this commit, please take a look there, otherwise the PR looks good to be merged to me. |
|
@damagatchi retest this please |
Product Description
Following points are covered in this PR
credentialstable with new fieldscredentialstable(Discard previous PR for this Api call for credentials and insert data in table #3202)3 tickets work is combined in this PR
https://dimagi.atlassian.net/browse/CCCT-1337
https://dimagi.atlassian.net/browse/CCCT-1340
https://dimagi.atlassian.net/browse/CCCT-1341
Note->
Credentialsapi is not yet deployed from backend so just added its implementation and not testedNew design ScreenShots:
Technical Summary
Feature Flag
Safety Assurance
Safety story
Automated test coverage
QA Plan
Labels and Review