MOSIP-44485:ARC - Export packet to local device#699
Open
damodarguru wants to merge 4 commits intomosip:developfrom
Open
MOSIP-44485:ARC - Export packet to local device#699damodarguru wants to merge 4 commits intomosip:developfrom
damodarguru wants to merge 4 commits intomosip:developfrom
Conversation
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
WalkthroughAdds an end-to-end ExportPacket UI test and ExportPage page object, registers the test in TestNG and TestRunner, introduces Regproc auth config and a KernelAuthentication helper, adds Kannada and Tamil document-upload page objects, adjusts biometric/scan locators and preview lookup, and includes several test-flow, retry, and formatting tweaks across UI tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as ExportPacket Test
participant Page as ExportPage
participant Driver as AppiumDriver
participant Picker as Device File Picker / Storage
Test->>Page: start exportPacket flow (login, language, consent)
Page->>Driver: navigate to Documents -> packets -> PACKET_MANAGER_ACCOUNT
Driver-->>Page: packet list displayed
Page->>Driver: open export action for packet
Page->>Picker: open folder chooser
Picker-->>Page: folder list
alt folder not present
Page->>Picker: create new folder (name)
Picker-->>Page: folder created
end
Page->>Picker: select folder and confirm "Use this folder"
Picker-->>Page: selection confirmed
Page->>Driver: trigger export into chosen folder
Driver-->>Page: export completed / success acknowledgement
Page->>Test: report export result and logout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
ui-test/src/main/java/regclient/pages/english/BiometricDetailsPageEnglish.java (1)
100-124: Local variables shadow class fields — clean up the now-unused fields.
clickOnLeftHandScanIcon,clickOnThumbsScanIcon, andclickOnFaceScanIconeach declare a local variable with the same name as the corresponding@AndroidFindByclass field (Lines 42–49). The fields are now dead code. Also,clickOnRightHandScanIcon()(Line 95) still uses the old field-based approach, creating an inconsistency.Consider:
- Removing the unused fields (
leftHandScanIcon,thumbsScanIcon,faceScanIcon).- Applying the same dynamic lookup pattern to
clickOnRightHandScanIcon()for consistency.♻️ Suggested cleanup for unused fields
- `@AndroidFindBy`(uiAutomator = "new UiScrollable(new UiSelector().scrollable(true).instance(0)) .scrollIntoView(new UiSelector().descriptionContains(\"Left\"))") - private WebElement leftHandScanIcon; - - `@AndroidFindBy`(uiAutomator = "new UiScrollable(new UiSelector().scrollable(true).instance(0)) .scrollIntoView(new UiSelector().descriptionContains(\"Thumbs\"))") - private WebElement thumbsScanIcon; - - `@AndroidFindBy`(uiAutomator = "new UiScrollable(new UiSelector().scrollable(true).instance(0)) .scrollIntoView(new UiSelector().descriptionContains(\"Face\"))") - private WebElement faceScanIcon;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/pages/english/BiometricDetailsPageEnglish.java` around lines 100 - 124, Remove the dead `@AndroidFindBy` fields leftHandScanIcon, thumbsScanIcon, and faceScanIcon (they’re shadowed by local variables in clickOnLeftHandScanIcon, clickOnThumbsScanIcon, clickOnFaceScanIcon) and update clickOnRightHandScanIcon to use the same dynamic MobileBy.AndroidUIAutomator scrollIntoView lookup pattern used in clickOnLeftHandScanIcon/clickOnThumbsScanIcon/clickOnFaceScanIcon so all four methods consistently find elements at runtime rather than relying on the now-removed `@AndroidFindBy` fields.ui-test/src/main/java/regclient/pages/english/SettingsPageEnglish.java (1)
85-86: Minor naming inconsistency: field/method names still reference "ScanNow" while the locator text is now "Scan".The accessibility value was updated to
"Scan", but the field (scanNowButton) and the methods (isScanNowButtonDisplayed,clickOnScanNowButton) still use the "ScanNow" naming. Consider renaming toscanButton/isScanButtonDisplayed/clickOnScanButtonfor consistency with the new label and with other pages (e.g.,ApplicantBiometricsPageEnglishalready usesscanButton).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/pages/english/SettingsPageEnglish.java` around lines 85 - 86, Rename the inconsistent identifiers to match the updated locator: change the field scanNowButton to scanButton and update any related methods isScanNowButtonDisplayed and clickOnScanNowButton to isScanButtonDisplayed and clickOnScanButton respectively; update all references/usages in SettingsPageEnglish so the `@AndroidFindBy`(accessibility = "Scan") private WebElement scanButton and its accessor/click methods are consistently named to match ApplicantBiometricsPageEnglish and the new accessibility text.ui-test/src/main/java/regclient/androidTestCases/ResetPassword.java (1)
109-112: Clarify the doubleclickOnLogoutButton()call.Lines 109 and 111 both invoke
profilePage.clickOnLogoutButton(). If the first click opens a confirmation dialog and the second dismisses/confirms it, consider using distinct method names to make the intent clear. If this is a workaround for a flaky element, a brief comment would help future maintainers understand the rationale.The hardcoded
waitTime(5)on Line 112 is also worth noting — an explicit wait for a specific condition (e.g., login page visibility) would be more robust than a fixed sleep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/androidTestCases/ResetPassword.java` around lines 109 - 112, The double call to profilePage.clickOnLogoutButton() is unclear and the fixed BasePage.waitTime(5) is brittle; replace the duplicated call with explicit actions and an explicit wait: if the first click opens a confirmation dialog, split into profilePage.clickLogoutButton() and profilePage.confirmLogout() (or profilePage.dismissLogoutConfirmation()/acceptLogoutConfirmation() as appropriate) and remove the hard sleep, using an explicit wait like profilePage.waitForLoginPageVisible() or BasePage.waitForElementVisible(loginPage.getLoginForm()) to wait for the login page instead of BasePage.waitTime(5); alternatively add a short comment explaining the flakiness if you must keep the workaround.ui-test/src/main/java/regclient/pages/english/PreviewPageEnglish.java (1)
53-54: ThebiometricCorrectionTitlefield (Line 53–54) is now unused.
isBiometricCorrectionTitleDisplayed()now performs its own dynamic lookup, making the@AndroidFindByfield dead code. Same pattern as thescanButtonfield inApplicantBiometricsPageEnglish.♻️ Suggested cleanup
- `@AndroidFindBy`(accessibility = "Biometric correction") - private WebElement biometricCorrectionTitle;Also applies to: 107-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/pages/english/PreviewPageEnglish.java` around lines 53 - 54, Remove the dead `@AndroidFindBy` field biometricCorrectionTitle since isBiometricCorrectionTitleDisplayed() performs a dynamic lookup; locate and delete the declaration "private WebElement biometricCorrectionTitle" and any other unused `@AndroidFindBy` fields in this class (the similar block around lines 107–118) to avoid dead code (follow the same cleanup pattern used for scanButton in ApplicantBiometricsPageEnglish); ensure no remaining references to biometricCorrectionTitle exist and run tests.ui-test/src/main/java/regclient/pages/english/ApplicantBiometricsPageEnglish.java (1)
62-63: ThescanButtonfield (Line 62) is now unused — consider removing it.
clickOnScanButton()now performs its own dynamic element lookup viaMobileBy.AndroidUIAutomator, making the@AndroidFindByfield at Line 62 dead code. Removing it avoids unnecessary page-factory initialization overhead and potential confusion.♻️ Suggested cleanup
- `@AndroidFindBy`(uiAutomator = "new UiScrollable(new UiSelector().scrollable(true).instance(0)).scrollIntoView(new UiSelector().description(\"Scan\"))") - private WebElement scanButton; -Also applies to: 111-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/pages/english/ApplicantBiometricsPageEnglish.java` around lines 62 - 63, Remove the dead page-factory fields that are no longer referenced (e.g., the `@AndroidFindBy-annotated` scanButton and any other similar fields in the same class around lines 111-117); update ApplicantBiometricsPageEnglish by deleting the scanButton field declaration and the other unused `@AndroidFindBy` fields, and confirm clickOnScanButton() (which uses MobileBy.AndroidUIAutomator) is the only locator used—also run a quick search in the class to ensure no remaining references to those field names before committing.ui-test/src/main/java/regclient/androidTestCases/LostUin.java (1)
548-557: Stabilize the upload retry check with a short wait/poll per attempt.On Line 551, the no-network check happens immediately after clicking upload. If the error appears with delay,
uploadSuccesscan be set incorrectly. Add a bounded wait (or polling) before deciding each attempt outcome.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/androidTestCases/LostUin.java` around lines 548 - 557, The upload retry loop checks isNoNetworkFoundDisplayed() immediately after manageApplicationsPage.clickOnUploadButton(), which can miss a delayed error; add a bounded wait or polling inside each iteration (e.g., poll isNoNetworkFoundDisplayed() with a short sleep and timeout or use an explicit wait utility) before deciding the attempt outcome so uploadSuccess is set only after the wait/poll completes; update the loop surrounding clickOnUploadButton() and isNoNetworkFoundDisplayed() to perform this per-attempt wait and then break or continue accordingly.ui-test/src/main/java/regclient/androidTestCases/ExportPacket.java (1)
553-557: Add a direct assertion that export actually completed.After Line 556, the flow continues without validating an export-success signal (toast/message/state). A positive completion assertion would prevent false-green runs when folder selection succeeds but export does not.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/androidTestCases/ExportPacket.java` around lines 553 - 557, Add a direct assertion after calling exportPage.exportPacketIntoFolderIfReady("ExportPacket") to verify the export actually completed: call an ExportPage method that checks for success (e.g. isExportSuccessful(), hasExportToast(), or getExportSuccessMessage()) or assert the exported file exists in the target folder; if such a method doesn't exist, implement one in ExportPage to detect the success toast/message or verify file creation and then assert its positive result immediately after exportPacketIntoFolderIfReady returns.ui-test/src/main/java/regclient/page/ExportPage.java (1)
166-196: Align this API with the fixedExportPacketfolder design.
exportPacketIntoFolder(String folderName)currently accepts arbitrary folder names, which makes accidental misuse easier. If this flow is intentionally fixed to one folder, prefer an internal constant and remove the parameterized surface.Based on learnings: In
ui-test/src/main/java/regclient/page/ExportPage.java, packet export is intentionally designed to always use theExportPacketfolder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/page/ExportPage.java` around lines 166 - 196, Change the export flow to always use a fixed ExportPacket folder: add a private static final String EXPORT_PACKET_FOLDER = "ExportPacket" (or the agreed folder name) and replace exportPacketIntoFolder(String folderName) with a no-arg exportPacketIntoFolder() that uses EXPORT_PACKET_FOLDER everywhere (calls to isFolderDisplayed, enterFolderName, selectFolderByName, logger messages, clickOnUseThisFolderButton, handleAllowFolderConsentIfPresent). Update exportPacketIntoFolderIfReady() to call the new no-arg exportPacketIntoFolder(); remove the external folderName parameter from the public API and ensure any internal helper calls (isFolderDisplayed, selectFolderByName) are invoked with EXPORT_PACKET_FOLDER.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui-test/src/main/java/regclient/androidTestCases/ExportPacket.java`:
- Around line 222-386: The loop in ExportPacket that iterates over screenOrder
silently ignores unknown screen keys (in the block handling screens like
"consentdet"/"DemographicDetails"/"BiometricDetails"), so add a final else
branch after the existing if/else-if chain that immediately fails the test
(throw AssertionError or call assertTrue(false, ...)) with a clear message
including the unexpected screen name and the language; update the method
containing this loop (the ExportPacket test method) to include this check so any
unhandled/mandatory screen in screenOrder causes an immediate, descriptive
failure.
In `@ui-test/src/main/java/regclient/page/ExportPage.java`:
- Around line 167-171: When the folder exists the code currently calls
clickOnUseThisFolderButton() without first opening the target folder, which may
select the wrong directory; update the if branch so that after
isFolderDisplayed(folderName) returns true you first open/select the folder
(e.g. call a helper like openFolder(folderName) or clickOnFolder(folderName))
and then call clickOnUseThisFolderButton() followed by
handleAllowFolderConsentIfPresent(); ensure the new helper clicks the folder
element corresponding to folderName before the “Use this folder” step.
In `@ui-test/src/main/resources/config/Kernel.properties`:
- Around line 36-39: ArcConfigManager is missing getter methods referenced by
KernelAuthentication; add methods getRegprocAppId(), getRegprocClientId(),
getRegprocClientSecret(), and the analogous
getIdrepoAppId()/getIdrepoClientId()/getIdrepoClientSecret() and
getAdminAppId()/getAdminClientId()/getAdminClientSecret() following the existing
getProperty(String) pattern so each returns the appropriate property value (use
keys mosip_regprocclient_app_id, mosip_regproc_client_id,
mosip_regproc_client_secret for regproc; mosip_idrepo_app_id,
mosip_idrepo_client_id, mosip_idrepo_client_secret for idrepo;
mosip_admin_app_id, mosip_admin_client_id, mosip_admin_client_secret for admin).
Also adjust or document the Kernel.properties key name inconsistency for regproc
(mosip_regprocclient_app_id vs mosip_regproc_client_id) so your getters use the
exact keys present in the properties file.
---
Nitpick comments:
In `@ui-test/src/main/java/regclient/androidTestCases/ExportPacket.java`:
- Around line 553-557: Add a direct assertion after calling
exportPage.exportPacketIntoFolderIfReady("ExportPacket") to verify the export
actually completed: call an ExportPage method that checks for success (e.g.
isExportSuccessful(), hasExportToast(), or getExportSuccessMessage()) or assert
the exported file exists in the target folder; if such a method doesn't exist,
implement one in ExportPage to detect the success toast/message or verify file
creation and then assert its positive result immediately after
exportPacketIntoFolderIfReady returns.
In `@ui-test/src/main/java/regclient/androidTestCases/LostUin.java`:
- Around line 548-557: The upload retry loop checks isNoNetworkFoundDisplayed()
immediately after manageApplicationsPage.clickOnUploadButton(), which can miss a
delayed error; add a bounded wait or polling inside each iteration (e.g., poll
isNoNetworkFoundDisplayed() with a short sleep and timeout or use an explicit
wait utility) before deciding the attempt outcome so uploadSuccess is set only
after the wait/poll completes; update the loop surrounding clickOnUploadButton()
and isNoNetworkFoundDisplayed() to perform this per-attempt wait and then break
or continue accordingly.
In `@ui-test/src/main/java/regclient/androidTestCases/ResetPassword.java`:
- Around line 109-112: The double call to profilePage.clickOnLogoutButton() is
unclear and the fixed BasePage.waitTime(5) is brittle; replace the duplicated
call with explicit actions and an explicit wait: if the first click opens a
confirmation dialog, split into profilePage.clickLogoutButton() and
profilePage.confirmLogout() (or
profilePage.dismissLogoutConfirmation()/acceptLogoutConfirmation() as
appropriate) and remove the hard sleep, using an explicit wait like
profilePage.waitForLoginPageVisible() or
BasePage.waitForElementVisible(loginPage.getLoginForm()) to wait for the login
page instead of BasePage.waitTime(5); alternatively add a short comment
explaining the flakiness if you must keep the workaround.
In `@ui-test/src/main/java/regclient/page/ExportPage.java`:
- Around line 166-196: Change the export flow to always use a fixed ExportPacket
folder: add a private static final String EXPORT_PACKET_FOLDER = "ExportPacket"
(or the agreed folder name) and replace exportPacketIntoFolder(String
folderName) with a no-arg exportPacketIntoFolder() that uses
EXPORT_PACKET_FOLDER everywhere (calls to isFolderDisplayed, enterFolderName,
selectFolderByName, logger messages, clickOnUseThisFolderButton,
handleAllowFolderConsentIfPresent). Update exportPacketIntoFolderIfReady() to
call the new no-arg exportPacketIntoFolder(); remove the external folderName
parameter from the public API and ensure any internal helper calls
(isFolderDisplayed, selectFolderByName) are invoked with EXPORT_PACKET_FOLDER.
In
`@ui-test/src/main/java/regclient/pages/english/ApplicantBiometricsPageEnglish.java`:
- Around line 62-63: Remove the dead page-factory fields that are no longer
referenced (e.g., the `@AndroidFindBy-annotated` scanButton and any other similar
fields in the same class around lines 111-117); update
ApplicantBiometricsPageEnglish by deleting the scanButton field declaration and
the other unused `@AndroidFindBy` fields, and confirm clickOnScanButton() (which
uses MobileBy.AndroidUIAutomator) is the only locator used—also run a quick
search in the class to ensure no remaining references to those field names
before committing.
In
`@ui-test/src/main/java/regclient/pages/english/BiometricDetailsPageEnglish.java`:
- Around line 100-124: Remove the dead `@AndroidFindBy` fields leftHandScanIcon,
thumbsScanIcon, and faceScanIcon (they’re shadowed by local variables in
clickOnLeftHandScanIcon, clickOnThumbsScanIcon, clickOnFaceScanIcon) and update
clickOnRightHandScanIcon to use the same dynamic MobileBy.AndroidUIAutomator
scrollIntoView lookup pattern used in
clickOnLeftHandScanIcon/clickOnThumbsScanIcon/clickOnFaceScanIcon so all four
methods consistently find elements at runtime rather than relying on the
now-removed `@AndroidFindBy` fields.
In `@ui-test/src/main/java/regclient/pages/english/PreviewPageEnglish.java`:
- Around line 53-54: Remove the dead `@AndroidFindBy` field
biometricCorrectionTitle since isBiometricCorrectionTitleDisplayed() performs a
dynamic lookup; locate and delete the declaration "private WebElement
biometricCorrectionTitle" and any other unused `@AndroidFindBy` fields in this
class (the similar block around lines 107–118) to avoid dead code (follow the
same cleanup pattern used for scanButton in ApplicantBiometricsPageEnglish);
ensure no remaining references to biometricCorrectionTitle exist and run tests.
In `@ui-test/src/main/java/regclient/pages/english/SettingsPageEnglish.java`:
- Around line 85-86: Rename the inconsistent identifiers to match the updated
locator: change the field scanNowButton to scanButton and update any related
methods isScanNowButtonDisplayed and clickOnScanNowButton to
isScanButtonDisplayed and clickOnScanButton respectively; update all
references/usages in SettingsPageEnglish so the `@AndroidFindBy`(accessibility =
"Scan") private WebElement scanButton and its accessor/click methods are
consistently named to match ApplicantBiometricsPageEnglish and the new
accessibility text.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
ui-test/pom.xmlui-test/src/main/java/regclient/androidTestCases/ExportPacket.javaui-test/src/main/java/regclient/androidTestCases/LostUin.javaui-test/src/main/java/regclient/androidTestCases/ResetPassword.javaui-test/src/main/java/regclient/androidTestCases/UpdateMyUinUpdateDemographicDetails.javaui-test/src/main/java/regclient/api/KernelAuthentication.javaui-test/src/main/java/regclient/page/ExportPage.javaui-test/src/main/java/regclient/page/KeycloakPage.javaui-test/src/main/java/regclient/pages/english/ApplicantBiometricsPageEnglish.javaui-test/src/main/java/regclient/pages/english/BiometricDetailsPageEnglish.javaui-test/src/main/java/regclient/pages/english/DocumentUploadPageEnglish.javaui-test/src/main/java/regclient/pages/english/ManageApplicationsPageEnglish.javaui-test/src/main/java/regclient/pages/english/PreviewPageEnglish.javaui-test/src/main/java/regclient/pages/english/SettingsPageEnglish.javaui-test/src/main/java/regclient/pages/kannada/DocumentUploadPageKannada.javaui-test/src/main/java/regclient/pages/tamil/DocumentUploadPageTamil.javaui-test/src/main/java/regclient/utils/TestRunner.javaui-test/src/main/resources/config/Kernel.propertiesui-test/src/main/resources/testdata.jsonui-test/testng.xml
💤 Files with no reviewable changes (1)
- ui-test/src/main/java/regclient/page/KeycloakPage.java
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
ui-test/src/main/java/regclient/page/ExportPage.java (1)
166-170: Open the target folder before tapping "Use this folder" (existing folder branch).When
isFolderTitleDisplayed(folderName)is true,clickOnUseThisFolderButton()is called without first enteringfolderName. This can apply "Use this folder" to the currently open directory instead of the intended subfolder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/page/ExportPage.java` around lines 166 - 170, The current exportPacketIntoFolder flow calls clickOnUseThisFolderButton() when isFolderTitleDisplayed(folderName) is true but never opens/enters that folder first, risking applying "Use this folder" to the wrong directory; update exportPacketIntoFolder to open/enter the target folder before clicking "Use this folder" (e.g., call a method like openFolder(folderName) or clickOnFolderTitle(folderName) prior to clickOnUseThisFolderButton()), and if such a helper does not exist, implement a clear openFolderByName(String folderName) that navigates into the folder, then call handleAllowFolderConsentIfPresent() as before.
🧹 Nitpick comments (3)
ui-test/src/main/java/regclient/page/ExportPage.java (3)
36-46: Three fields share the sameandroid:id/button1locator — consolidate or document intent.
okButton(line 37),useThisFolderButton(line 40), andallowFolderButton(line 46) all resolve to the same element at runtime. While they appear in different dialog contexts, duplicating the same@AndroidFindBymapping across three fields adds noise and risks confusion if a locator ever diverges. Consider using a single shared field (e.g.,primaryActionButton) or at minimum add comments noting they are context-specific aliases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/page/ExportPage.java` around lines 36 - 46, Multiple fields (okButton, useThisFolderButton, allowFolderButton) use the same `@AndroidFindBy`(id = "android:id/button1") locator; consolidate to a single shared field (e.g., primaryActionButton) and update callers to use that, or if aliases are required keep one canonical field and convert the others to documented getters/wrappers that return the canonical WebElement; reference okButton, useThisFolderButton, allowFolderButton and accessFolderAlertPopup when making the change so dialog context is preserved and add a short comment explaining that the single locator is reused across different dialogs.
33-34:newfolderPopupandaccessFolderAlertPopupshare the samecom.google.android.documentsui:id/alertTitlelocator.Both fields resolve to the same element. If these are meant to distinguish between two different dialogs, the locator strategy won't achieve that. If they are the same popup in different contexts, a single field suffices.
Also applies to: 42-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/page/ExportPage.java` around lines 33 - 34, The two page fields newfolderPopup and accessFolderAlertPopup in ExportPage both use the identical locator "com.google.android.documentsui:id/alertTitle" so they always resolve to the same element; either consolidate them into a single field if they represent the same dialog, or change the locator strategy to uniquely identify each dialog (for example use a more specific XPath/CSS that includes the dialog container or visible text, or locate a parent element and then the title) and update the PageFactory annotations accordingly; also apply the same fix for the similar duplicate at the other occurrence referenced in the comment.
132-159:isPacketManagerTitleDisplayed()navigates as a side effect — consider renaming or splitting.An
is...predicate that also clicks through three folders (Documents → packets → PACKET_MANAGER_ACCOUNT) violates the principle of least surprise. Callers expecting a pure boolean check receive navigation as a bonus, which makes the method hard to reuse and debug. Consider splitting it into anavigateToPacketManagerFolder()method and a pureisPacketManagerTitleDisplayed()predicate, or renaming the current method tonavigateToPacketManagerAndVerify().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/page/ExportPage.java` around lines 132 - 159, The method isPacketManagerTitleDisplayed() performs navigation (clicks/scrolls) as a side effect; split responsibilities by extracting the navigation logic into a new method (e.g., navigateToPacketManagerFolder()) that contains the scrollToText and clickOnElement calls for documentsFolder, packetsFolder and packetManagerAccountFolder and preserves the existing try/catch and logging, then change isPacketManagerTitleDisplayed() to be a pure predicate that only checks and returns isElementDisplayed(packetManagerTitle); alternatively rename the current method to navigateToPacketManagerAndVerify() if you prefer to keep navigation+check together and update all callers to use the new navigate method and the pure predicate where appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui-test/src/main/java/regclient/page/ExportPage.java`:
- Around line 119-130: The log messages in handleAllowFolderConsentIfPresent
incorrectly say "Access consent" (copied from handleAccessConsentIfPresent);
update all three logger.info calls in handleAllowFolderConsentIfPresent to
reference "Allow folder consent" (or "Allow folder consent not displayed" /
"Allow folder consent not present. Continuing flow.") so logs accurately reflect
the accessFolderAlertPopup and allowFolderButton actions and the exception path.
- Around line 104-107: The clickOnOkButton method currently calls
driver.navigate().back() which navigates away and breaks the following flow in
exportPacketIntoFolder (selectFolderByName and clickOnUseThisFolderButton);
remove the unconditional driver.navigate().back() from clickOnOkButton and, if
the intent was to dismiss an occasional consent dialog, replace it by invoking a
conditional handler such as handleAllowFolderConsentIfPresent() from
exportPacketIntoFolder (or call that handler immediately after clickOnOkButton)
so subsequent calls to selectFolderByName and clickOnUseThisFolderButton operate
on the correct screen.
- Around line 109-113: selectFolderByName and isFolderTitleDisplayed inline
folderName into UIAutomator/XPath strings and will break if folderName contains
quotes or parentheses; update both methods (selectFolderByName,
isFolderTitleDisplayed) to produce quote-safe selectors by escaping/quoting
folderName rather than simple concatenation: implement a small helper that
returns a safe literal for UIAutomator/XPath (e.g., choose single-quote or
double-quote wrapping when safe, or build an XPath concat(...) expression when
the name contains both quotes) and use that helper when creating folderLocator
and the XPath in isFolderTitleDisplayed so generated selectors are always valid
for arbitrary folderName values.
---
Duplicate comments:
In `@ui-test/src/main/java/regclient/page/ExportPage.java`:
- Around line 166-170: The current exportPacketIntoFolder flow calls
clickOnUseThisFolderButton() when isFolderTitleDisplayed(folderName) is true but
never opens/enters that folder first, risking applying "Use this folder" to the
wrong directory; update exportPacketIntoFolder to open/enter the target folder
before clicking "Use this folder" (e.g., call a method like
openFolder(folderName) or clickOnFolderTitle(folderName) prior to
clickOnUseThisFolderButton()), and if such a helper does not exist, implement a
clear openFolderByName(String folderName) that navigates into the folder, then
call handleAllowFolderConsentIfPresent() as before.
---
Nitpick comments:
In `@ui-test/src/main/java/regclient/page/ExportPage.java`:
- Around line 36-46: Multiple fields (okButton, useThisFolderButton,
allowFolderButton) use the same `@AndroidFindBy`(id = "android:id/button1")
locator; consolidate to a single shared field (e.g., primaryActionButton) and
update callers to use that, or if aliases are required keep one canonical field
and convert the others to documented getters/wrappers that return the canonical
WebElement; reference okButton, useThisFolderButton, allowFolderButton and
accessFolderAlertPopup when making the change so dialog context is preserved and
add a short comment explaining that the single locator is reused across
different dialogs.
- Around line 33-34: The two page fields newfolderPopup and
accessFolderAlertPopup in ExportPage both use the identical locator
"com.google.android.documentsui:id/alertTitle" so they always resolve to the
same element; either consolidate them into a single field if they represent the
same dialog, or change the locator strategy to uniquely identify each dialog
(for example use a more specific XPath/CSS that includes the dialog container or
visible text, or locate a parent element and then the title) and update the
PageFactory annotations accordingly; also apply the same fix for the similar
duplicate at the other occurrence referenced in the comment.
- Around line 132-159: The method isPacketManagerTitleDisplayed() performs
navigation (clicks/scrolls) as a side effect; split responsibilities by
extracting the navigation logic into a new method (e.g.,
navigateToPacketManagerFolder()) that contains the scrollToText and
clickOnElement calls for documentsFolder, packetsFolder and
packetManagerAccountFolder and preserves the existing try/catch and logging,
then change isPacketManagerTitleDisplayed() to be a pure predicate that only
checks and returns isElementDisplayed(packetManagerTitle); alternatively rename
the current method to navigateToPacketManagerAndVerify() if you prefer to keep
navigation+check together and update all callers to use the new navigate method
and the pure predicate where appropriate.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ui-test/src/main/java/regclient/page/ExportPage.javaui-test/src/main/java/regclient/pages/english/DocumentUploadPageEnglish.javaui-test/src/main/java/regclient/pages/kannada/DocumentUploadPageKannada.javaui-test/src/main/java/regclient/pages/tamil/DocumentUploadPageTamil.java
✅ Files skipped from review due to trivial changes (1)
- ui-test/src/main/java/regclient/pages/english/DocumentUploadPageEnglish.java
Signed-off-by: damodarguru <damodar.g@cyberpwn.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
ui-test/src/main/java/regclient/page/ExportPage.java (2)
108-114:⚠️ Potential issue | 🟡 MinorPartial escape fix — backslash not pre-escaped.
selectFolderByNamenow escapes"→\"but does not first escape\→\\. A name likeFoo\Barwould produceFoo\Barin the UIAutomator string instead ofFoo\\Bar, which is a malformed selector at runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/page/ExportPage.java` around lines 108 - 114, In selectFolderByName, the input only escapes double quotes but not backslashes, so names like "Foo\Bar" produce malformed UIAutomator strings; fix by first escaping backslashes (replace "\" with "\\") then escape double quotes (replace "\"" with "\\\"") to produce a properly escaped safeName, and use that safeName when building the MobileBy.AndroidUIAutomator selector in the method.
167-171:⚠️ Potential issue | 🟠 Major
selectFolderByName(folderName)is still missing beforeclickOnUseThisFolderButton()in the existing-folder branch.When the folder already exists the code confirms its presence via
isFolderTitleDisplayedbut never navigates into it;clickOnUseThisFolderButton()then operates on the current (parent) directory rather than the target subfolder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/regclient/page/ExportPage.java` around lines 167 - 171, In exportPacketIntoFolder, when the folder exists (isFolderTitleDisplayed(folderName) branch) you must call selectFolderByName(folderName) before clickOnUseThisFolderButton() so the action targets the subfolder instead of the parent; update the branch in exportPacketIntoFolder to invoke selectFolderByName(folderName) then clickOnUseThisFolderButton() and finally handleAllowFolderConsentIfPresent().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui-test/src/main/java/regclient/page/ExportPage.java`:
- Around line 162-165: The scrollToText method inlines the raw text into the
AndroidUIAutomator string which breaks on quotes or backslashes; change
scrollToText (and similarly selectFolderByName) to escape backslashes and
double-quotes in the input first (e.g., replace "\" with "\\\\" then "\"" with
"\\\"") and then insert the escaped string into the UiAutomator expression so
the generated new UiSelector().text("...") is always valid; update the reference
in driver.findElement(MobileBy.AndroidUIAutomator(...)) to use the escaped
variable.
- Around line 36-46: Multiple fields (okButton, useThisFolderButton,
allowFolderButton) all use the same locator "android:id/button1" and
accessFolderAlertPopup (and newfolderPopup) share
"com.google.android.documentsui:id/alertTitle", which leads to caching and
incorrect mapping; fix by consolidating duplicate locators into a single
descriptive field (e.g., primaryAlertButton and alertTitle) and update call
sites (clickOnOkButton, clickOnUseThisFolderButton,
handleAllowFolderConsentIfPresent, isNewFolderPopupDisplayed) to use that
unified field, or if the buttons are distinct in different contexts replace the
duplicate `@AndroidFindBy` with context-specific locators (e.g.,
UiSelector.instance()/parent container constraints) to uniquely identify each
element before updating the same call sites to reference the new unique fields.
- Around line 179-181: After clickOnOkButton() you must navigate into the newly
created folder before calling clickOnUseThisFolderButton(); update the flow in
ExportPage so that after clickOnOkButton() you call
selectFolderByName(newFolderName) (using the same name passed when creating the
folder) and then proceed to clickOnUseThisFolderButton() and
handleAllowFolderConsentIfPresent(); alternatively refactor the branch to
perform an explicit folder selection/navigation rather than assuming DocumentsUI
auto-enters the new folder.
---
Duplicate comments:
In `@ui-test/src/main/java/regclient/page/ExportPage.java`:
- Around line 108-114: In selectFolderByName, the input only escapes double
quotes but not backslashes, so names like "Foo\Bar" produce malformed
UIAutomator strings; fix by first escaping backslashes (replace "\" with "\\")
then escape double quotes (replace "\"" with "\\\"") to produce a properly
escaped safeName, and use that safeName when building the
MobileBy.AndroidUIAutomator selector in the method.
- Around line 167-171: In exportPacketIntoFolder, when the folder exists
(isFolderTitleDisplayed(folderName) branch) you must call
selectFolderByName(folderName) before clickOnUseThisFolderButton() so the action
targets the subfolder instead of the parent; update the branch in
exportPacketIntoFolder to invoke selectFolderByName(folderName) then
clickOnUseThisFolderButton() and finally handleAllowFolderConsentIfPresent().
jayesh12234
approved these changes
Feb 26, 2026
mohanachandran-s
approved these changes
Feb 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MOSIP-44485:ARC - Export packet to local device
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Chores