Dashboard Cards: Store Dashboard Cards #2197
Conversation
The current 'CardsModel' structure doesn't fit the agreed database structure, where every card type will be inserted in its own row. As such, the structure is now changing to the new 'CardModel' sealed class structure, where every inner data class to become the a card model with a specific type, ultimately converting the 'cards' from a single data class, that holds in it all the individual cards, to a list of cards.
The current gson adapter cannot parse this specific date format on the post response as it doesn't contain the timezone information required. As such the post modified string date need to be manually converted through the new 'CardsUtils' to a date.
This is done in order to make the date a bit more timezone agnostic.
This 'post_date_gmt' value from the post response is actually the value that is going to be needed on the UI layer to at least show for when a scheduled post is due. As such, this is the main date that is going to be needed. However, in addition to that 'post_date_gmt', I decided to keep the 'post_modified_gmt' as well, use the 'modified' post model field for it, since it might end up being needed after all. But, mind the fact that it is not needed on the UI atm, the 'post_date_gmt' is enough as it is the only date to be used, and that will only applicable to scheduled posts.
This is done in order to keep the content grouped together.
After a discussion, it was decided that the previously added 'post_date_gmt' value from the post response is actually enough and as such keeping the 'post_modified_gmt' is not required.
After a discussion, it was decided that the post response needs to be updated with the newly agreed structure (a more compact post response). You will also notice that the 'title' and 'content' are now not-null. When there exist no title or content for a post the post response will return an empty string (instead of a null). This effectively make 'featured_image' the only nullable property for a post card. PS: The API endpoint has been already updated to adhere with the new requirements, which is to return a more compact list of posts, for more details see D69742.
This cards database access object (dao) will be used to store and retrieve the list of cards, from the database, for a specific site. Also, as part of this update, the 'CardsSqlUtils' is deleted as it will not be required at all.
These tests will fail as the implementation is not ready yet. The next commit will make the tests pass (TDD).
This commit makes the previous failing test pass (TDD).
These test will fail as the implementation is not ready yet. The next commit will make the tests pass (TDD).
This commit makes the previous failing test pass (TDD).
1) Convert to expression body. 2) Remove explicit type specification.
This includes upgrading both var 'type' and 'message' fields into val.
Those constants are actually used by the 'WPAndroidDatabaseMigrationTest' suite. Instead, this commit will suppress that with 'MemberVisibilityCanBePrivate' so that to get rid of this false positive IDE warning.
ParaskP7
changed the title
fluxc/src/main/java/org/wordpress/android/fluxc/model/dashboard/CardModel.kt
Show resolved
Hide resolved
fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpcom/dashboard/CardsRestClient.kt
Show resolved
Hide resolved
fluxc/src/main/java/org/wordpress/android/fluxc/store/dashboard/CardsStore.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Looking good @ParaskP7 . Walking us through your plan-of-attack (last week), was a big help while doing this review. 🙏 I left a couple of questions, but nothing that stops me from giving this the thumbs up, but I do want you to have the opportunity to think about the ❓ .
I would love to test this all the way through, but I always get stumped on applying the differential in my sandbox and then proxying my device. And I'm not sure if we really have time for your suggestion.
Let me know if you need any help with that!
Let us see what tomorrow brings. Other than that, 🙇 nice job!
|
👋 @zwarm !
Thanks so much for going through the review! 🥇
Thank you for sharing your thoughts, I have answered them to the best of my abilities. Let me know and we can definitely fine-tune all that as we arrive into an agreement. 🙇
Please do let me know and we can sit together to setup the sandbox and proxy for you. It really is not that big of a deal, I was also very afraid of that, but now that I am using it every day I know that it is just a matter of few minutes. And then, when this gets configured, it is a matter of few seconds to switch on the sandbox and connect the proxy. 💯 PS: I also think that this is a superpower, thus, the quicker you get it working for you, the better! |
zwarm
changed the title
Parent: WordPress-Android#15498
This PR:
To test:
CardsRestClientandCardsStoreclasses and make sure that the accompanying tests are working as expected.ReleaseStack_CardsTesttest suite gets implemented, then triggering this connected test suite will be another way of verifying that this solution works as expected.To manual test:
WordPress-AndroidPR to be created, which will implement the connection between the dashboard cards store and its source.Merge instructions:
Wait for the new API endpoint to be deployed (see D69742).[DO NOT MERGE]from the title.[PR] Not Ready For Mergelabel.