Fix Invalid removal after saving articles in history. #4745
Conversation
Bug : T366132
| @@ -45,22 +45,33 @@ interface ReadingListPageDao { | |||
| fun getPagesByStatus(status: Long): List<ReadingListPage> | |||
|
|
|||
| @Query("SELECT * FROM ReadingListPage WHERE wiki = :wiki AND lang = :lang AND namespace = :ns AND apiTitle = :apiTitle AND listId = :listId AND status != :excludedStatus") | |||
| fun getPageByParams(wiki: WikiSite, lang: String, ns: Namespace, | |||
| apiTitle: String, listId: Long, excludedStatus: Long): ReadingListPage? | |||
| fun getPageByParams( | |||
There was a problem hiding this comment.
Could you please update the pull request to contain only the code you are adding, without reformatting any other code? This will make it much clearer and easier to review.
| @@ -314,6 +326,8 @@ class SavedPageSyncService : JobIntentService() { | |||
| const val SUMMARY_PROGRESS = 10 | |||
| const val MEDIA_LIST_PROGRESS = 30 | |||
|
|
|||
| var pageToBeDeletedButNotSavedYet: MutableList<ReadingListPage>? = null | |||
There was a problem hiding this comment.
Architecturally it feels very uncomfortable to keep a global (static) object with a queue of pages for any reason. This is basically allowing database state to leak into a global context, and is almost guaranteed to be a source of bugs in the future.
Can you restructure the solution to make use of the database state of pages exclusively, and not rely on a separate global object?
There was a problem hiding this comment.
@dbrant I agree, that's not the best solution. That's why I clean up the value right after the operation.
I've tested several solutions, and I think the most suitable is to put the button in a loading or disabled state until the post is fully saved. This prevents the user from deleting it before the operation is complete.
What do you think? Do you have any suggestions?
Saving and removing articles are running asynchronously.
Removing an article just after saving it were creating an invalid state sometimes, the article was not deleted.
Bug : T366132