From 10c8e6e1a4964121c92d19e372cef68545f27a22 Mon Sep 17 00:00:00 2001 From: Sneha Datta Date: Thu, 3 Oct 2024 16:23:08 +0530 Subject: [PATCH 01/14] Fix #5404: Migrate away from onBackPressed for remaining activities (#5526) ## Explanation Fixes #5404 This PR migrates deprecated `onBackPressed` usage to `OnBackPressedDispatcher` callback in the following activities and presenters. - ProfileEditActivity - ProfileEditActivityPresenter - QuestionPlayerActivityPresenter - WalkthroughFinalFragmentPresenter ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> Co-authored-by: Mr. 17 --- .../app/settings/profile/ProfileEditActivity.kt | 15 ++++++++++++--- .../profile/ProfileEditActivityPresenter.kt | 3 +-- .../questionplayer/QuestionPlayerActivity.kt | 14 ++++++++++---- .../QuestionPlayerActivityPresenter.kt | 3 +-- .../app/walkthrough/WalkthroughActivity.kt | 14 ++++++++++---- .../end/WalkthroughFinalFragmentPresenter.kt | 3 +-- 6 files changed, 35 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivity.kt b/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivity.kt index c595bd367ca..2698cfe96a7 100644 --- a/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivity.kt +++ b/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivity.kt @@ -3,6 +3,7 @@ package org.oppia.android.app.settings.profile import android.content.Context import android.content.Intent import android.os.Bundle +import androidx.activity.OnBackPressedCallback import org.oppia.android.app.activity.ActivityComponentImpl import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity import org.oppia.android.app.model.ProfileEditActivityParams @@ -43,17 +44,25 @@ class ProfileEditActivity : InjectableAutoLocalizedAppCompatActivity() { super.onCreate(savedInstanceState) (activityComponent as ActivityComponentImpl).inject(this) profileEditActivityPresenter.handleOnCreate() + + onBackPressedDispatcher.addCallback( + this, + object : OnBackPressedCallback(/* enabled = */ true) { + override fun handleOnBackPressed() { + this@ProfileEditActivity.handleBackPress() + } + } + ) } - override fun onBackPressed() { + private fun handleBackPress() { val args = intent.getProtoExtra( PROFILE_EDIT_ACTIVITY_PARAMS_KEY, ProfileEditActivityParams.getDefaultInstance() ) val isMultipane = args?.isMultipane ?: false if (isMultipane) { - @Suppress("DEPRECATION") // TODO(#5404): Migrate to a back pressed dispatcher. - super.onBackPressed() + finish() } else { val intent = Intent(this, ProfileListActivity::class.java) intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP) diff --git a/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivityPresenter.kt index ea67fb89474..b89f8857de2 100644 --- a/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivityPresenter.kt @@ -31,8 +31,7 @@ class ProfileEditActivityPresenter @Inject constructor( toolbar.setNavigationOnClickListener { if (isMultipane) { - @Suppress("DEPRECATION") // TODO(#5404): Migrate to a back pressed dispatcher. - activity.onBackPressed() + activity.onBackPressedDispatcher.onBackPressed() } else { val intent = Intent(activity, ProfileListActivity::class.java) intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP) diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivity.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivity.kt index 70239733952..027cbd7011f 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivity.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivity.kt @@ -3,6 +3,7 @@ package org.oppia.android.app.topic.questionplayer import android.content.Context import android.content.Intent import android.os.Bundle +import androidx.activity.OnBackPressedCallback import org.oppia.android.app.activity.ActivityComponentImpl import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity import org.oppia.android.app.hintsandsolution.HintsAndSolutionListener @@ -56,11 +57,16 @@ class QuestionPlayerActivity : val profileId = intent.extractCurrentUserProfileId() questionPlayerActivityPresenter.handleOnCreate(profileId) - } - override fun onBackPressed() { - showStopExplorationDialogFragment() - questionPlayerActivityPresenter.setReadingTextSizeNormal() + onBackPressedDispatcher.addCallback( + this, + object : OnBackPressedCallback(/* enabled = */ true) { + override fun handleOnBackPressed() { + showStopExplorationDialogFragment() + questionPlayerActivityPresenter.setReadingTextSizeNormal() + } + } + ) } override fun restartSession() = questionPlayerActivityPresenter.restartSession() diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt index 523ebf2cc4d..345941e95e1 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt @@ -61,8 +61,7 @@ class QuestionPlayerActivityPresenter @Inject constructor( activity.setSupportActionBar(binding.questionPlayerToolbar) binding.questionPlayerToolbar.setNavigationOnClickListener { - @Suppress("DEPRECATION") // TODO(#5404): Migrate to a back pressed dispatcher. - activity.onBackPressed() + activity.onBackPressedDispatcher.onBackPressed() } retrieveReadingTextSize().observe( diff --git a/app/src/main/java/org/oppia/android/app/walkthrough/WalkthroughActivity.kt b/app/src/main/java/org/oppia/android/app/walkthrough/WalkthroughActivity.kt index 083b293bad7..d3d7a7081e3 100644 --- a/app/src/main/java/org/oppia/android/app/walkthrough/WalkthroughActivity.kt +++ b/app/src/main/java/org/oppia/android/app/walkthrough/WalkthroughActivity.kt @@ -3,6 +3,7 @@ package org.oppia.android.app.walkthrough import android.content.Context import android.content.Intent import android.os.Bundle +import androidx.activity.OnBackPressedCallback import org.oppia.android.app.activity.ActivityComponentImpl import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity import org.oppia.android.app.model.ProfileId @@ -22,6 +23,15 @@ class WalkthroughActivity : super.onCreate(savedInstanceState) (activityComponent as ActivityComponentImpl).inject(this) walkthroughActivityPresenter.handleOnCreate() + + onBackPressedDispatcher.addCallback( + this, + object : OnBackPressedCallback(/* enabled = */ true) { + override fun handleOnBackPressed() { + walkthroughActivityPresenter.handleSystemBack() + } + } + ) } override fun currentPage(walkthroughPage: Int) { @@ -33,10 +43,6 @@ class WalkthroughActivity : walkthroughActivityPresenter.changePage(walkthroughPage) } - override fun onBackPressed() { - walkthroughActivityPresenter.handleSystemBack() - } - companion object { fun createWalkthroughActivityIntent(context: Context, internalProfileId: Int): Intent { diff --git a/app/src/main/java/org/oppia/android/app/walkthrough/end/WalkthroughFinalFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/walkthrough/end/WalkthroughFinalFragmentPresenter.kt index abd286f77d8..078d79af431 100644 --- a/app/src/main/java/org/oppia/android/app/walkthrough/end/WalkthroughFinalFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/walkthrough/end/WalkthroughFinalFragmentPresenter.kt @@ -103,7 +103,6 @@ class WalkthroughFinalFragmentPresenter @Inject constructor( } override fun goBack() { - @Suppress("DEPRECATION") // TODO(#5404): Migrate to a back pressed dispatcher. - activity.onBackPressed() + activity.onBackPressedDispatcher.onBackPressed() } } From 5e140e989949bee89b1b266b535608bd2f839615 Mon Sep 17 00:00:00 2001 From: Sneha Datta Date: Wed, 9 Oct 2024 15:06:52 +0530 Subject: [PATCH 02/14] Fix #5404: Migrate away from onBackPressed for RevisionCardActivity (#5548) ## Explanation Fixes #5404 This PR migrates deprecated `onBackPressed `usage to `OnBackPressedDispatcher` callback in the RevisonCardActivity and RevisionCardActivityPresenter. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --- .../topic/revisioncard/RevisionCardActivity.kt | 17 ++++++++++------- .../RevisionCardActivityPresenter.kt | 3 +-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivity.kt b/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivity.kt index 7606c7ea5ad..1dc30c6fd9e 100644 --- a/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivity.kt +++ b/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivity.kt @@ -3,6 +3,7 @@ package org.oppia.android.app.topic.revisioncard import android.content.Context import android.content.Intent import android.os.Bundle +import androidx.activity.OnBackPressedCallback import org.oppia.android.app.activity.ActivityComponentImpl import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity import org.oppia.android.app.model.ProfileId @@ -56,6 +57,15 @@ class RevisionCardActivity : subtopicListSize ) } + onBackPressedDispatcher.addCallback( + this, + object : OnBackPressedCallback(/* enabled = */ true) { + override fun handleOnBackPressed() { + revisionCardActivityPresenter.setReadingTextSizeMedium() + onReturnToTopicRequested() + } + } + ) } override fun handleOnOptionsItemSelected(itemId: Int) { @@ -115,13 +125,6 @@ class RevisionCardActivity : revisionCardActivityPresenter.dismissConceptCard() } - // TODO(#5404): Migrate to a back pressed dispatcher. - @Deprecated("Deprecated in Java") - override fun onBackPressed() { - revisionCardActivityPresenter.setReadingTextSizeMedium() - onReturnToTopicRequested() - } - override fun onDefaultFontSizeLoaded(readingTextSize: ReadingTextSize) { revisionCardActivityPresenter.loadRevisionCardFragment(readingTextSize) } diff --git a/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt index af41963e498..c77afb97738 100644 --- a/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt @@ -83,8 +83,7 @@ class RevisionCardActivityPresenter @Inject constructor( binding.revisionCardToolbar.setNavigationOnClickListener { (activity as ReturnToTopicClickListener).onReturnToTopicRequested() fontScaleConfigurationUtil.adjustFontScale(activity, ReadingTextSize.MEDIUM_TEXT_SIZE) - @Suppress("DEPRECATION") // TODO(#5404): Migrate to a back pressed dispatcher. - activity.onBackPressed() + activity.onBackPressedDispatcher.onBackPressed() } if (!accessibilityService.isScreenReaderEnabled()) { binding.revisionCardToolbarTitle.setOnClickListener { From f1d576dfed3b144daf6e9651d070b3d95cc0d63e Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 17:13:33 +0530 Subject: [PATCH 03/14] check todo works proper --- .../ApplicationLifecycleObserverTest.kt | 1 + .../android/scripts/common/GitHubClient.kt | 5 +- .../scripts/common/remote/GitHubService.kt | 4 +- .../android/scripts/todo/TodoOpenCheckTest.kt | 62 +++++++++++++++++-- 4 files changed, 66 insertions(+), 6 deletions(-) diff --git a/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt b/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt index 48e1ccc01c4..4af3734ee5d 100644 --- a/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt @@ -427,6 +427,7 @@ class ApplicationLifecycleObserverTest { ) // TODO(#5341): Replace appSessionId generation to the modified Twitter snowflake algorithm. + // TODO(#5240): Replace appSessionId generation to the modified Twitter snowflake algorithm. val sessionIdProvider = loggingIdentifierController.getAppSessionId() val sessionId = monitorFactory.waitForNextSuccessfulResult(sessionIdProvider) diff --git a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt index fd7cc105037..0931fd27f74 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt @@ -72,7 +72,10 @@ class GitHubClient( "Failed to fetch issues at page $pageNumber: ${response.code()}\n${call.request()}" + "\n${response.errorBody()}." } - return@async checkNotNull(response.body()) { + //subha + // Filter out any pull requests from the list added + val issues = response.body()?.filter { it.pullRequest == null } + return@async checkNotNull(issues) { "No issues response from GitHub for page: $pageNumber." } } diff --git a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt index 0fdb1dd3a19..94ef2d330cf 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt @@ -29,7 +29,9 @@ interface GitHubService { * @return the list of [GitHubIssue]s read from the remote repository (as a [Call]) */ @Headers("Accept: application/vnd.github+json", "X-GitHub-Api-Version: 2022-11-28") - @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") + //@GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") + //add by subha + @GET("repos/{repo_owner}/{repo_name}/issues?state=open&sort=created&direction=asc") fun fetchOpenIssues( @Path("repo_owner") repoOwner: String, @Path("repo_name") repoName: String, diff --git a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt index b614bc361d7..6d3e6a5b4a9 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -84,6 +84,35 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(TODO_CHECK_PASSED_OUTPUT_INDICATOR) } + //subha + @Test + fun testOpenIssuesOnly_withPullRequests_checkShouldFail() { + // Setup with open issues and some pull requests + setUpGitHubService(openIssueNumbers = listOf(11004, 11003, 11002, 11001), pullRequestNumbers = listOf(21001)) + + val tempFile1 = tempFolder.newFile("testfiles/TempFile1.kt") + val tempFile2 = tempFolder.newFile("testfiles/TempFile2.kt") + val testContent1 = + """ + // TODO(#11002): test summary 1. + # TODO(#11004): test summary 2. + test Todo + test TODO + """.trimIndent() + val testContent2 = + """ + // TODO(#21001): test summary 3. + todo + + """.trimIndent() + tempFile1.writeText(testContent1) + tempFile2.writeText(testContent2) + + runScript() + + assertThat(outContent.toString().trim()).isEqualTo(TODO_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) + } + @Test fun testTodoCheck_onlyPoorlyFormattedTodosPresent_checkShouldFail() { setUpGitHubService(issueNumbers = emptyList()) @@ -777,14 +806,39 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(failureMessage) } - private fun setUpGitHubService(issueNumbers: List) { - val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } + //subha , just adding a parameter pull request + private fun setUpGitHubService(openIssueNumbers: List, pullRequestNumbers: List = emptyList()) { + // Create JSON objects for open issues + val openIssuesJson = openIssueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } + + // Create JSON objects for pull requests (using the `pull_request` field to indicate it's a PR) + val pullRequestsJson = pullRequestNumbers.joinToString(separator = ",") { + "{\"number\":$it, \"pull_request\":{}}" // Include an empty pull_request object + } + + // Combine them to simulate a typical GitHub response where issues & PRs might mix + val combinedJson = if (pullRequestsJson.isNotEmpty()) { + "[$openIssuesJson, $pullRequestsJson]" + } else { + "[$openIssuesJson]" // Only include open issues if PRs are empty + } val mockWebServer = MockWebServer() - mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) - mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. + + mockWebServer.enqueue(MockResponse().setBody(combinedJson)) + mockWebServer.enqueue(MockResponse().setBody("[]")) // Simulate end of pagination. + + // Set the mock server URL for the GitHub client to use GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() } +// private fun setUpGitHubService(issueNumbers: List) { +// val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } +// val mockWebServer = MockWebServer() +// mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) +// mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. +// GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() +// } + private fun setUpSupportForGhAuth(authToken: String) { fakeCommandExecutor.registerHandler("gh") { _, args, outputStream, _ -> when (args) { From fbf524927da93f2c78f7e5de6ee7107c269ac5a1 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 17:16:10 +0530 Subject: [PATCH 04/14] correct formatting --- .../org/oppia/android/scripts/common/GitHubClient.kt | 2 +- .../android/scripts/common/remote/GitHubService.kt | 4 ++-- .../oppia/android/scripts/todo/TodoOpenCheckTest.kt | 10 ++++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt index 0931fd27f74..e1670101db5 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt @@ -72,7 +72,7 @@ class GitHubClient( "Failed to fetch issues at page $pageNumber: ${response.code()}\n${call.request()}" + "\n${response.errorBody()}." } - //subha + // subha // Filter out any pull requests from the list added val issues = response.body()?.filter { it.pullRequest == null } return@async checkNotNull(issues) { diff --git a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt index 94ef2d330cf..21d6f475c09 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt @@ -29,8 +29,8 @@ interface GitHubService { * @return the list of [GitHubIssue]s read from the remote repository (as a [Call]) */ @Headers("Accept: application/vnd.github+json", "X-GitHub-Api-Version: 2022-11-28") - //@GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") - //add by subha + // @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") + // add by subha @GET("repos/{repo_owner}/{repo_name}/issues?state=open&sort=created&direction=asc") fun fetchOpenIssues( @Path("repo_owner") repoOwner: String, diff --git a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt index 6d3e6a5b4a9..08325a15626 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -84,11 +84,12 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(TODO_CHECK_PASSED_OUTPUT_INDICATOR) } - //subha + // subha @Test fun testOpenIssuesOnly_withPullRequests_checkShouldFail() { // Setup with open issues and some pull requests - setUpGitHubService(openIssueNumbers = listOf(11004, 11003, 11002, 11001), pullRequestNumbers = listOf(21001)) + setUpGitHubService(openIssueNumbers = listOf(11004, 11003, 11002, 11001), + pullRequestNumbers = listOf(21001)) val tempFile1 = tempFolder.newFile("testfiles/TempFile1.kt") val tempFile2 = tempFolder.newFile("testfiles/TempFile2.kt") @@ -806,8 +807,9 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(failureMessage) } - //subha , just adding a parameter pull request - private fun setUpGitHubService(openIssueNumbers: List, pullRequestNumbers: List = emptyList()) { + // subha + private fun setUpGitHubService(openIssueNumbers: List, + pullRequestNumbers: List = emptyList()) { // Create JSON objects for open issues val openIssuesJson = openIssueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } From de91d663e48fb9f25f7ad2b6aab446dcd6f51234 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 17:19:16 +0530 Subject: [PATCH 05/14] correct format --- .../oppia/android/scripts/todo/TodoOpenCheckTest.kt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt index 08325a15626..6f5f5ada375 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -88,8 +88,9 @@ class TodoOpenCheckTest { @Test fun testOpenIssuesOnly_withPullRequests_checkShouldFail() { // Setup with open issues and some pull requests - setUpGitHubService(openIssueNumbers = listOf(11004, 11003, 11002, 11001), - pullRequestNumbers = listOf(21001)) + setUpGitHubService( + openIssueNumbers = listOf(11004, 11003, 11002, 11001), pullRequestNumbers = listOf(21001) + ) val tempFile1 = tempFolder.newFile("testfiles/TempFile1.kt") val tempFile2 = tempFolder.newFile("testfiles/TempFile2.kt") @@ -808,8 +809,9 @@ class TodoOpenCheckTest { } // subha - private fun setUpGitHubService(openIssueNumbers: List, - pullRequestNumbers: List = emptyList()) { + private fun setUpGitHubService( + openIssueNumbers: List, pullRequestNumbers: List = emptyList() + ) { // Create JSON objects for open issues val openIssuesJson = openIssueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } From f26b7be43489c3992c8f311f0e3babf2d86bb060 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 17:20:31 +0530 Subject: [PATCH 06/14] hi --- .../org/oppia/android/scripts/todo/TodoOpenCheckTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt index 6f5f5ada375..afa645fed4f 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -810,7 +810,8 @@ class TodoOpenCheckTest { // subha private fun setUpGitHubService( - openIssueNumbers: List, pullRequestNumbers: List = emptyList() + openIssueNumbers: List, + pullRequestNumbers: List = emptyList() ) { // Create JSON objects for open issues val openIssuesJson = openIssueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } From 6034262154fab20941b816e910885552b56423e6 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 20:05:18 +0530 Subject: [PATCH 07/14] hi --- .../org/oppia/android/scripts/common/remote/GitHubService.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt index 21d6f475c09..ba1d90b11f9 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt @@ -31,7 +31,8 @@ interface GitHubService { @Headers("Accept: application/vnd.github+json", "X-GitHub-Api-Version: 2022-11-28") // @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") // add by subha - @GET("repos/{repo_owner}/{repo_name}/issues?state=open&sort=created&direction=asc") + @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") + //@GET("repos/{repo_owner}/{repo_name}/issues?state=open&direction=asc") fun fetchOpenIssues( @Path("repo_owner") repoOwner: String, @Path("repo_name") repoName: String, From 2cf4bed6de383ed9b7f112ac0cdd718499715ded Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 20:06:12 +0530 Subject: [PATCH 08/14] hi --- .../org/oppia/android/scripts/common/remote/GitHubService.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt index ba1d90b11f9..5895c12ceae 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt @@ -32,7 +32,7 @@ interface GitHubService { // @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") // add by subha @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") - //@GET("repos/{repo_owner}/{repo_name}/issues?state=open&direction=asc") + // @GET("repos/{repo_owner}/{repo_name}/issues?state=open&direction=asc") fun fetchOpenIssues( @Path("repo_owner") repoOwner: String, @Path("repo_name") repoName: String, From 7c093affa55a106fe849b4caf27e83bbcea91c32 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 20:35:23 +0530 Subject: [PATCH 09/14] add pull req field --- .../oppia/android/scripts/common/model/GitHubIssue.kt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt b/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt index 7a824fc3120..98ef437430d 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt @@ -10,4 +10,13 @@ import com.squareup.moshi.JsonClass * 'issues/' in an issue's GitHub URL) */ @JsonClass(generateAdapter = true) -data class GitHubIssue(@Json(name = "number") val number: Int) +data class GitHubIssue( + @Json(name = "number") val number: Int, + @Json(name = "pull_request") val pullRequest: PullRequest? = null // Nullable field to distinguish pull requests +) + +// Define PullRequest class as needed, or leave it empty if you don’t require specific details +@JsonClass(generateAdapter = true) +data class PullRequest( + // Add fields if needed, or leave empty for filtering purposes +) \ No newline at end of file From bc55b891724f3f157e81ba143f30f948a1f21c87 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 20:36:50 +0530 Subject: [PATCH 10/14] hi --- .../java/org/oppia/android/scripts/common/model/GitHubIssue.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt b/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt index 98ef437430d..208223d2542 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt @@ -12,7 +12,7 @@ import com.squareup.moshi.JsonClass @JsonClass(generateAdapter = true) data class GitHubIssue( @Json(name = "number") val number: Int, - @Json(name = "pull_request") val pullRequest: PullRequest? = null // Nullable field to distinguish pull requests + @Json(name = "pull_request") val pullRequest: PullRequest? = null ) // Define PullRequest class as needed, or leave it empty if you don’t require specific details From d4c849e8d7e4c1efc2b7281bfc447761c28788d2 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 20:39:41 +0530 Subject: [PATCH 11/14] hi --- .../java/org/oppia/android/scripts/common/model/GitHubIssue.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt b/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt index 208223d2542..76de5cabdb1 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt @@ -19,4 +19,4 @@ data class GitHubIssue( @JsonClass(generateAdapter = true) data class PullRequest( // Add fields if needed, or leave empty for filtering purposes -) \ No newline at end of file +) From 81d091298a1e11011d9f4fa02128834e227cef97 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 21:04:20 +0530 Subject: [PATCH 12/14] hi --- .../org/oppia/android/scripts/common/GitHubClient.kt | 5 +---- .../oppia/android/scripts/common/model/GitHubIssue.kt | 11 +---------- .../android/scripts/common/remote/GitHubService.kt | 7 ++++++- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt index e1670101db5..fd7cc105037 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt @@ -72,10 +72,7 @@ class GitHubClient( "Failed to fetch issues at page $pageNumber: ${response.code()}\n${call.request()}" + "\n${response.errorBody()}." } - // subha - // Filter out any pull requests from the list added - val issues = response.body()?.filter { it.pullRequest == null } - return@async checkNotNull(issues) { + return@async checkNotNull(response.body()) { "No issues response from GitHub for page: $pageNumber." } } diff --git a/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt b/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt index 76de5cabdb1..7a824fc3120 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt @@ -10,13 +10,4 @@ import com.squareup.moshi.JsonClass * 'issues/' in an issue's GitHub URL) */ @JsonClass(generateAdapter = true) -data class GitHubIssue( - @Json(name = "number") val number: Int, - @Json(name = "pull_request") val pullRequest: PullRequest? = null -) - -// Define PullRequest class as needed, or leave it empty if you don’t require specific details -@JsonClass(generateAdapter = true) -data class PullRequest( - // Add fields if needed, or leave empty for filtering purposes -) +data class GitHubIssue(@Json(name = "number") val number: Int) diff --git a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt index 5895c12ceae..92d1e6e8cc2 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt @@ -31,7 +31,12 @@ interface GitHubService { @Headers("Accept: application/vnd.github+json", "X-GitHub-Api-Version: 2022-11-28") // @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") // add by subha - @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") + @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc&pulls=false") + suspend fun fetchIssues( + @Path("repo_owner") repoOwner: String, + @Path("repo_name") repoName: String + ): Response> + // @GET("repos/{repo_owner}/{repo_name}/issues?state=open&direction=asc") fun fetchOpenIssues( @Path("repo_owner") repoOwner: String, From 6b8576e6b959cc3332bf98752d1e2c12f255e17e Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 21:14:23 +0530 Subject: [PATCH 13/14] hi --- .../org/oppia/android/scripts/common/remote/GitHubService.kt | 5 ----- 1 file changed, 5 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt index 92d1e6e8cc2..8a01cafcea1 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt @@ -32,11 +32,6 @@ interface GitHubService { // @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") // add by subha @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc&pulls=false") - suspend fun fetchIssues( - @Path("repo_owner") repoOwner: String, - @Path("repo_name") repoName: String - ): Response> - // @GET("repos/{repo_owner}/{repo_name}/issues?state=open&direction=asc") fun fetchOpenIssues( @Path("repo_owner") repoOwner: String, From f3bb778313e2653f5ba6fe94f2fdb833bae73b76 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 31 Oct 2024 22:56:20 +0530 Subject: [PATCH 14/14] hi --- .../scripts/common/remote/GitHubService.kt | 2 - .../android/scripts/todo/TodoOpenCheckTest.kt | 67 ++----------------- 2 files changed, 4 insertions(+), 65 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt index 8a01cafcea1..5240843f895 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt @@ -29,10 +29,8 @@ interface GitHubService { * @return the list of [GitHubIssue]s read from the remote repository (as a [Call]) */ @Headers("Accept: application/vnd.github+json", "X-GitHub-Api-Version: 2022-11-28") - // @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") // add by subha @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc&pulls=false") - // @GET("repos/{repo_owner}/{repo_name}/issues?state=open&direction=asc") fun fetchOpenIssues( @Path("repo_owner") repoOwner: String, @Path("repo_name") repoName: String, diff --git a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt index afa645fed4f..b614bc361d7 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -84,37 +84,6 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(TODO_CHECK_PASSED_OUTPUT_INDICATOR) } - // subha - @Test - fun testOpenIssuesOnly_withPullRequests_checkShouldFail() { - // Setup with open issues and some pull requests - setUpGitHubService( - openIssueNumbers = listOf(11004, 11003, 11002, 11001), pullRequestNumbers = listOf(21001) - ) - - val tempFile1 = tempFolder.newFile("testfiles/TempFile1.kt") - val tempFile2 = tempFolder.newFile("testfiles/TempFile2.kt") - val testContent1 = - """ - // TODO(#11002): test summary 1. - # TODO(#11004): test summary 2. - test Todo - test TODO - """.trimIndent() - val testContent2 = - """ - // TODO(#21001): test summary 3. - todo - - """.trimIndent() - tempFile1.writeText(testContent1) - tempFile2.writeText(testContent2) - - runScript() - - assertThat(outContent.toString().trim()).isEqualTo(TODO_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) - } - @Test fun testTodoCheck_onlyPoorlyFormattedTodosPresent_checkShouldFail() { setUpGitHubService(issueNumbers = emptyList()) @@ -808,42 +777,14 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(failureMessage) } - // subha - private fun setUpGitHubService( - openIssueNumbers: List, - pullRequestNumbers: List = emptyList() - ) { - // Create JSON objects for open issues - val openIssuesJson = openIssueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } - - // Create JSON objects for pull requests (using the `pull_request` field to indicate it's a PR) - val pullRequestsJson = pullRequestNumbers.joinToString(separator = ",") { - "{\"number\":$it, \"pull_request\":{}}" // Include an empty pull_request object - } - - // Combine them to simulate a typical GitHub response where issues & PRs might mix - val combinedJson = if (pullRequestsJson.isNotEmpty()) { - "[$openIssuesJson, $pullRequestsJson]" - } else { - "[$openIssuesJson]" // Only include open issues if PRs are empty - } + private fun setUpGitHubService(issueNumbers: List) { + val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } val mockWebServer = MockWebServer() - - mockWebServer.enqueue(MockResponse().setBody(combinedJson)) - mockWebServer.enqueue(MockResponse().setBody("[]")) // Simulate end of pagination. - - // Set the mock server URL for the GitHub client to use + mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) + mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() } -// private fun setUpGitHubService(issueNumbers: List) { -// val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } -// val mockWebServer = MockWebServer() -// mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) -// mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. -// GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() -// } - private fun setUpSupportForGhAuth(authToken: String) { fakeCommandExecutor.registerHandler("gh") { _, args, outputStream, _ -> when (args) {