Skip to content

Commit

Permalink
Fix oppia#3317: Update old todos (oppia#3610)
Browse files Browse the repository at this point in the history
* Fix oppia#3317: Update old todos

* Remove resolved todos

* Repurpose todos stage 1

* Repurpose todos stage 2

* updating todos part 3

* update more todos

* add network module

* update todo no. 322

* Remove unused imports

* Revert "Remove unused imports"

This reverts commit 65ee6e9.

* Revert "update todo no. 322"

This reverts commit c150cdc.

* Repurpose no. 322

* replace issue number for backend model todo

* link new filed issue

Co-authored-by: Sparsh Agrawal <[email protected]>
  • Loading branch information
Sparsh1212 and Sparsh Agrawal committed Aug 6, 2021
1 parent 3f98e53 commit 2b33802
Show file tree
Hide file tree
Showing 33 changed files with 40 additions and 49 deletions.
2 changes: 1 addition & 1 deletion app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ dependencies {
kaptAndroidTest(
'com.google.dagger:dagger-compiler:2.24'
)
// TODO (#59): Remove this once Bazel is introduced
// TODO(#59): Remove this once Bazel is introduced
api project(':data')
implementation project(":model")
testImplementation project(":model")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class ExitProfileDialogFragment : DialogFragment() {
dialog.dismiss()
}
.setPositiveButton(R.string.home_activity_back_dialog_exit) { _, _ ->
// TODO(#322): Need to start intent for ProfileChooserActivity to get update. Change to finish when live data bug is fixed.
// TODO(#3641): Investigate on using finish instead of intent.
val intent = ProfileChooserActivity.createProfileChooserActivity(activity!!)
if (!restoreLastCheckedItem) {
intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class NavigationDrawerFragmentPresenter @Inject constructor(

// TODO(#3382): Remove debug only code from prod build (also check imports, constructor and drawer_fragment.xml)
private fun setIfDeveloperOptionsMenuItemListener() {
// TODO(3383): Find a way to make this work below N
// TODO(#3383): Find a way to make this work below N
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
developerOptionsStarter.ifPresent { starter ->
getFooterViewModel().isDebugMode.set(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class OngoingListAdapter(

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RecyclerView.ViewHolder {
return when (viewType) {
// TODO(#216): Generalize this binding to make adding future items easier.
// TODO(#632): Generalize this binding to make adding future items easier.
VIEW_TYPE_SECTION_TITLE_TEXT -> {
val inflater = LayoutInflater.from(parent.context)
val binding =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ class SelectionInteractionViewModel(
}
return false
} else if (selectedItems.size < maxAllowableSelectionCount) {
// TODO(#32): Add warning to user when they exceed the number of allowable selections or are under the minimum
// TODO(#3624): Add warning to user when they exceed the number of allowable selections or are under the minimum
// number required.
selectedItems += itemIndex
val wasSelectedItemListEmpty = isAnswerAvailable.get()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class AddProfileActivity : InjectableAppCompatActivity() {
}

override fun onSupportNavigateUp(): Boolean {
// TODO(#322): Need to start intent for ProfileChooserActivity to get update. Change to finish when live data bug is fixed.
// TODO(#3641): Investigate on using finish instead of intent.
val intent = Intent(this, ProfileChooserActivity::class.java)
intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
startActivity(intent)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule
import org.oppia.android.app.player.state.hintsandsolution.HintsAndSolutionConfigModule
import org.oppia.android.app.shim.ViewBindingShimModule
import org.oppia.android.app.topic.PracticeTabModule
import org.oppia.android.data.backends.gae.NetworkModule
import org.oppia.android.domain.classify.InteractionsModule
import org.oppia.android.domain.classify.rules.continueinteraction.ContinueModule
import org.oppia.android.domain.classify.rules.dragAndDropSortInput.DragDropSortInputModule
Expand Down Expand Up @@ -107,12 +108,12 @@ class HelpActivityTest {
}

// TODO(#59): Figure out a way to reuse modules instead of needing to re-declare them.
// TODO(#1675): Add NetworkModule once data module is migrated off of Moshi.
@Singleton
@Component(
modules = [
RobolectricModule::class,
PlatformParameterModule::class,
NetworkModule::class,
TestDispatcherModule::class, ApplicationModule::class,
LoggerModule::class, ContinueModule::class, FractionInputModule::class,
ItemSelectionInputModule::class, MultipleChoiceInputModule::class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ import org.robolectric.annotation.LooperMode
import javax.inject.Inject
import javax.inject.Singleton

// TODO #1815: Remove these duplicate values once Screenshot tests are implemented.
// TODO(#1815): Remove these duplicate values once Screenshot tests are implemented.
private const val SMALL_TEXT_SIZE_SCALE = 0.8f
private const val MEDIUM_TEXT_SIZE_SCALE = 1.0f
private const val LARGE_TEXT_SIZE_SCALE = 1.2f
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ class NavigationDrawerActivityProdTest {
}
}

// TODO(#1806): Enable this once lowfi implementation is done.
// TODO(#552): Enable this once lowfi implementation is done.
// TODO(#2535): Unable to open NavigationDrawer multiple times on Robolectric
@RunOn(TestPlatform.ESPRESSO)
@Test
Expand Down Expand Up @@ -465,7 +465,7 @@ class NavigationDrawerActivityProdTest {
}
}

// TODO(#1806): Enable this once lowfi implementation is done.
// TODO(#552): Enable this once lowfi implementation is done.
// TODO(#2535): Unable to open NavigationDrawer multiple times on Robolectric
@RunOn(TestPlatform.ESPRESSO)
@Test
Expand Down Expand Up @@ -547,7 +547,7 @@ class NavigationDrawerActivityProdTest {
}
}

// TODO(#1806): Enable this once lowfi implementation is done.
// TODO(#552): Enable this once lowfi implementation is done.
// TODO(#2535): Unable to open NavigationDrawer multiple times on Robolectric
@RunOn(TestPlatform.ESPRESSO)
@Test
Expand Down Expand Up @@ -631,7 +631,7 @@ class NavigationDrawerActivityProdTest {
}
}

// TODO(#1806): Enable this once lowfi implementation is done.
// TODO(#552): Enable this once lowfi implementation is done.
// TODO(#2535): Unable to open NavigationDrawer multiple times on Robolectric
@RunOn(TestPlatform.ESPRESSO)
@Test
Expand Down Expand Up @@ -753,7 +753,7 @@ class NavigationDrawerActivityProdTest {
}
}

// TODO(#1806): Enable this once lowfi implementation is done.
// TODO(#552): Enable this once lowfi implementation is done.
@Test
@Ignore("My Downloads is removed until we have full download support.")
fun testNavDrawer_myDownloadsMenu_myDownloadsFragmentIsDisplayed() {
Expand Down
4 changes: 2 additions & 2 deletions data/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ dependencies {
'org.robolectric:robolectric:4.4',
project(":testing"),
)
// TODO (#59): Remove this once Bazel is introduced
// TODO (#97): Isolate retrofit-mock dependency from production
// TODO(#59): Remove this once Bazel is introduced
// TODO(#97): Isolate retrofit-mock dependency from production
api(
'com.squareup.retrofit2:converter-moshi:2.5.0',
'com.squareup.retrofit2:retrofit:2.5.0',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ object NetworkSettings {
private var isDeveloperMode: Boolean = true

/** DEVELOPER URL which connects to development server */
// TODO(#74): Move this to DI graph
// TODO(#3623): Move this to DI graph
private const val DEVELOPER_URL = "https://oppia.org"
/** PRODUCTION URL which connects to production server */
private const val PROD_URL = "https://oppia.org"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass

/** Data class for the feedback report sent by the Android app to remote storage. */
// TODO(#2801): Link backend domain model
// TODO(#3016): Link backend domain model
@JsonClass(generateAdapter = true)
data class GaeFeedbackReport(

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass

/** Data class for feedback reporting app info represented in the backend domain model. */
// TODO(#2801): Link backend domain model
// TODO(#3016): Link backend domain model
@JsonClass(generateAdapter = true)
data class GaeFeedbackReportingAppContext(

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass

/** Data class for feedback reporting device build model. */
// TODO(#2801): Link backend domain model
// TODO(#3016): Link backend domain model
@JsonClass(generateAdapter = true)
data class GaeFeedbackReportingDeviceContext(

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass

/** Data class for the feedback reporting entry point represented in the backend storage model. */
// TODO(#2801): Link backend domain model
// TODO(#3016): Link backend domain model
@JsonClass(generateAdapter = true)
data class GaeFeedbackReportingEntryPoint(

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass

/** Data class for feedback reporting device information model. */
// TODO(#2801): Link backend domain model
// TODO(#3016): Link backend domain model
@JsonClass(generateAdapter = true)
data class GaeFeedbackReportingSystemContext(

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass

/** Data class for feedback reporting form model. */
// TODO(#2801): Link backend domain model
// TODO(#3016): Link backend domain model
@JsonClass(generateAdapter = true)
data class GaeUserSuppliedFeedback(

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ class ExplorationProgressController @Inject constructor(
private val oppiaLogger: OppiaLogger
) {
// TODO(#179): Add support for parameters.
// TODO(#182): Add support for refresher explorations.
// TODO(#90): Update the internal locking of this controller to use something like an in-memory
// TODO(#3622): Update the internal locking of this controller to use something like an in-memory
// blocking cache to simplify state locking. However, doing this correctly requires a fix in
// MediatorLiveData to avoid unexpected cancellations in chained cross-scope coroutines. Note
// that this is also essential to ensure post-load operations can be queued before load completes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ class LogStorageModule {
@EventLogStorageCacheSize
fun provideEventLogStorageCacheSize(): Int = 5000

/** Provides the number of records that can be stored in ExceptionLog's cache storage.*/
// TODO (#1104): Add correct number of records and size calculations for exceptions.
/**
* Provides the number of records that can be stored in ExceptionLog's cache storage.
* The current [ExceptionLogStorageCacheSize] is set to be 25 records.
* Taking 130 bytes per record, it is expected to occupy around 3250 bytes of disk space.
*/
@Provides
@ExceptionLogStorageCacheSize
fun provideExceptionLogStorageCacheSize(): Int = 25
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class AnalyticsController @Inject constructor(
.addEventLog(eventLog)
.build()
} else {
// TODO (#1433): Refactoring for logging exceptions to both console and exception loggers.
// TODO(#1433): Refactoring for logging exceptions to both console and exception loggers.
val exception =
NullPointerException("Least Recent Event index absent -- EventLogCacheStoreSize is 0")
consoleLogger.e("Analytics Controller", exception.toString())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class ExceptionsController @Inject constructor(
.addExceptionLog(exceptionLog)
.build()
} else {
// TODO (#1433): Refactoring for logging exceptions to both console and exception loggers.
// TODO(#1433): Refactoring for logging exceptions to both console and exception loggers.
val exception =
NullPointerException(
"Least Recent Exception index absent -- ExceptionLogCacheStoreSize is 0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ class StoryProgressController @Inject constructor(
private val dataProviders: DataProviders,
private val oppiaLogger: OppiaLogger
) {
// TODO(#21): Determine whether chapters can have missing prerequisites in the initial prototype,
// or if that just indicates that they can't be started due to previous chapter not yet being
// completed.

/** These Statuses correspond to the exceptions above such that if the deferred contains. */
private enum class StoryProgressActionStatus {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ class TopicController @Inject constructor(
}
}

// TODO(#21): Expose this as a data provider, or omit if it's not needed.
internal fun retrieveTopic(topicId: String): Topic {
return if (loadLessonProtosFromAssets) {
val topicRecord =
Expand Down Expand Up @@ -474,7 +473,7 @@ class TopicController @Inject constructor(

private fun computeTopicSizeBytes(constituentFiles: List<String>): Int {
// TODO(#169): Compute this based on protos & the combined topic package.
// TODO(#386): Incorporate image files in this computation.
// TODO(#169): Incorporate image files in this computation.
return constituentFiles.map { file ->
if (loadLessonProtosFromAssets) {
assetRepository.getLocalAssetProtoSize(file)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ private const val INVALID_EXPLORATION_ID = "invalid_exp_id"
@LooperMode(LooperMode.Mode.PAUSED)
@Config(application = ExplorationProgressControllerTest.TestApplication::class)
class ExplorationProgressControllerTest {
// TODO(#114): Add much more thorough tests for the integration pathway.
// TODO(#3646): Add much more thorough tests for the integration pathway.

// TODO(#59): Once AsyncDataSubscriptionManager can be replaced with a fake, add the following
// tests once careful testing timing can be controlled:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ class AnalyticsControllerTest {
.isEqualTo(ACTIVITYCONTEXT_NOT_SET)
}

// TODO(#1106): Addition of tests tracking behaviour of the controller after uploading of logs to the remote service.
// TODO(#3621): Addition of tests tracking behaviour of the controller after uploading of logs to the remote service.

@Test
fun testController_logTransitionEvent_withNoNetwork_checkLogsEventToStore() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class ExceptionsControllerTest {
assertThat(exceptionLogged).isEqualTo(exceptionThrown)
}

// TODO(#1106): Addition of tests tracking behaviour of the controller after uploading of logs to the remote service.
// TODO(#3621): Addition of tests tracking behaviour of the controller after uploading of logs to the remote service.

@Test
fun testController_logException_nonFatal_withNoNetwork_logsToCacheStore() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,6 @@ class QuestionAssessmentProgressControllerTest {
@Captor
lateinit var asyncAnswerOutcomeCaptor: ArgumentCaptor<AsyncResult<AnsweredQuestionOutcome>>

// TODO(#2738): Add tests for score and mastery calculations

@Test
fun testGetCurrentQuestion_noSessionStarted_returnsPendingResult() {
setUpTestApplicationWithSeed(questionSeed = 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,6 @@ class TopicListControllerTest {
ApplicationProvider.getApplicationContext<TestApplication>().inject(this)
}

// TODO(#15): Add tests for recommended lessons rather than promoted, and tests for the 'continue playing' LiveData
// not providing any data for cases when there are no ongoing lessons. Also, add tests for other uncovered cases
// (such as having and not having lessons in either of the PromotedActivityList section, or AsyncResult errors).

@Test
fun testRetrieveTopicList_isSuccessful() {
val topicListLiveData = topicListController.getTopicList().toLiveData()
Expand Down
2 changes: 1 addition & 1 deletion scripts/pre-push.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ else
exit 1
fi

# TODO(#1736): Add Bazel Linter to the project
# TODO(#3000): Add Bazel Linter to the project
# TODO(#970): Add XML Linter to the project
2 changes: 1 addition & 1 deletion testing/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ dependencies {
annotationProcessor(
'com.google.auto.service:auto-service:1.0-rc4',
)
// TODO (#59): Remove this once Bazel is introduced
// TODO(#59): Remove this once Bazel is introduced
// sufficiently visible for generated Dagger code. This can be done more cleanly via Bazel since dependencies can be
// controlled more directly than in Gradle.
implementation project(':model')
Expand Down
2 changes: 1 addition & 1 deletion utility/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ dependencies {
kaptTest(
'com.google.dagger:dagger-compiler:2.24'
)
// TODO (#59): Remove this once Bazel is introduced
// TODO(#59): Remove this once Bazel is introduced
// sufficiently visible for generated Dagger code. This can be done more cleanly via Bazel since dependencies can be
// controlled more directly than in Gradle.
implementation project(':model')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ class AsyncDataSubscriptionManager @Inject constructor(
* observers of the child ID.
*/
fun associateIds(childId: Any, parentId: Any) {
// TODO(#6): Ensure this graph is acyclic to avoid infinite recursion during notification. Compile-time deps should
// TODO(#3625): Ensure this graph is acyclic to avoid infinite recursion during notification. Compile-time deps should
// make this impossible in practice unless data provider users try to use the same key for multiple inter-dependent
// data providers.
// TODO(#6): Find a way to determine parent-child ID associations during subscription time to avoid needing to store
// TODO(#3625): Find a way to determine parent-child ID associations during subscription time to avoid needing to store
// long-lived references to IDs prior to subscriptions.
associatedIds.enqueue(parentId, childId)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import android.content.Context
* @param <T> The type of data being provided.
*/
abstract class DataProvider<T>(val context: Context) {
// TODO(#6): Finalize the interfaces for this API beyond a basic prototype for the initial project
// intro.

/**
* Returns a unique identifier that corresponds to this data provider. This should be a trivially
Expand Down

0 comments on commit 2b33802

Please sign in to comment.