change-history-backup#206
Conversation
migration added
# Conflicts: # respect-datalayer-db/schemas/world.respect.datalayer.db.RespectSchoolDatabase/12.json # respect-datalayer-db/src/commonMain/kotlin/world/respect/datalayer/db/RespectSchoolDatabase.kt # respect-datalayer-db/src/commonMain/kotlin/world/respect/datalayer/db/RespectSchoolDatabaseMigrations.kt # respect-datalayer-db/src/commonMain/kotlin/world/respect/datalayer/db/SchoolDataSourceDb.kt # respect-datalayer-http/src/commonMain/kotlin/world/respect/datalayer/http/SchoolDataSourceHttp.kt # respect-datalayer-repository/src/commonMain/kotlin/world/respect/datalayer/repository/SchoolDataSourceRepository.kt # respect-datalayer-repository/src/commonMain/kotlin/world/respect/datalayer/repository/school/writequeue/DrainRemoteWriteQueueUseCase.kt # respect-datalayer/src/commonMain/kotlin/world/respect/datalayer/SchoolDataSource.kt # respect-datalayer/src/commonMain/kotlin/world/respect/datalayer/SchoolDataSourceLocal.kt # respect-datalayer/src/commonMain/kotlin/world/respect/datalayer/school/writequeue/WriteQueueItem.kt
…f name change history.
|
its ready for review |
|
|
||
| if (historyGuidHashes.isEmpty()) return | ||
|
|
||
| schoolDb.getChangeHistoryDao().markByHistoryGuids(historyGuidHashes) } |
There was a problem hiding this comment.
what on earth is that closing bracket doing over there? Please... Tidy it.
| import world.respect.datalayer.school.model.ChangeHistoryTableEnum | ||
|
|
||
| interface ChangeHistoryProvider { | ||
| suspend fun getChangeHistoryEntries( |
There was a problem hiding this comment.
This would get only pending to send to server change history entries, correct? In which case, should be renamed accordingly (e.g. getPendingChangeHistoryEntries)
|
|
||
| changeHistoryFlow.collect { state -> | ||
|
|
||
| val changeHistoryList = state.dataOrNull()?.map { entry -> |
There was a problem hiding this comment.
This sorting should be done in the database using Sort by in the SQL.
| ) : RespectViewModel(savedStateHandle), KoinScopeComponent { | ||
| private val schoolDataSource: SchoolDataSource by inject() | ||
|
|
||
| override val scope: Scope = accountManager.requireActiveAccountScope() |
There was a problem hiding this comment.
The scope must go before use of inject, otherwise results can be unpredictable.
| prev.copy( | ||
| showAddStudent = selectedAccountAndPerson?.person?.isAdminOrTeacher() == true, | ||
| showAddTeacher = selectedAccountAndPerson?.person?.isAdminOrTeacher() == true, | ||
| changeHistoryButtonVisible = hasReadPermission, |
There was a problem hiding this comment.
This is not needed and does not make sense. If a person does not have read permission, they shouldn't be on this screen. And the datasource will enforce that they won't be able to read the class entity itself.
| permissionsRequiredByRole = CheckPersonPermissionUseCase.PermissionsRequiredByRole.WRITE_PERMISSIONS, | ||
| ) | ||
|
|
||
| val hasReadPermission = checkPersonPermissionUseCase( |
There was a problem hiding this comment.
Same as per comment on class: this check does not make sense.
| when (val incoming = call.receive<JsonElement>()) { | ||
|
|
||
| is JsonArray -> { | ||
| val clazz = Json.decodeFromJsonElement<List<Clazz>>(incoming) |
There was a problem hiding this comment.
Use json from the dependency injectoin
| ) | ||
| val schoolDataSource = schoolDataSource(call) | ||
|
|
||
| when (val incoming = call.receive<JsonElement>()) { |
There was a problem hiding this comment.
@Nikunjsharma0 #yellow this is a case of failure to follow Don't Repeat Youself principles as per code guidelines.
You should not have three copies of exactly the same logic. This must be done using an extension function or some other suitable code reuse mechanism.
There was a problem hiding this comment.
e.g. something like:
fun <T> ApplicationCall.receiveDataAndChangeHistory(
deserializer: DeserializationStrategy<T>
): DataAndChangeHistory<T> {
when...
}
3 participants
No description provided.