Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3317: Update old todos #3610

Merged
merged 16 commits into from
Aug 6, 2021
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.
BenHenning marked this conversation as resolved.
Show resolved Hide resolved
// 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