#298: Select active child #361
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The section was renamed to Add/Choose active profile, as suggested in https://preview.uxpin.com/dc276af793ed5a347736f473bdabe8c5112f8983#/pages/87956056/simulate/sitemap?mode=i . Tests were added but Espresso doesn't let them through, despite them working on production.
malsolec
suggested changes
Oct 7, 2018
|
|
||
| private void setActiveChild(){ | ||
| List<Child> childList = childRepository.getAll(); | ||
| for (Child child : childList) |
There was a problem hiding this comment.
could you please add brackets here? it would be easier to read.
| for (Child child : childList) | ||
| if(child.getIsActive()) | ||
| child.setIsActive(false); | ||
| childListAdapter.getChild(selectedChildPosition).setIsActive(true); |
There was a problem hiding this comment.
When I tested it the active value was not setting, you could select the row but it does not look that the value is stored anywhere. I don't think we update child in db and we should update the record after selected it with is_active to true. In my opinion it should also change the color of the row, when the list is displayed on opening child list view. That is probably implemented now, but one child should always be active and it should be visible on the list. The list should not allow to disactivate all children. Could you please verify it?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The ability to select an active child in "Add / remove child" section was added.
Section was renamed to "Add/Choose active child", as suggested in https://preview.uxpin.com/dc276af793ed5a347736f473bdabe8c5112f8983#/pages/87956056/simulate/sitemap?mode=i .
I once again struggle with Espresso though - tests were added but Espresso doesn't let them through, despite them working great on production.
Having two breakpoints, in line 110 and 111 of ChildListActivity.java (setActiveChild()), we can see, that if we set any child to active, we will hit the 111 line breakpoint. Then if we set another child to active (meaning we change the active child, as there can only be one at a time), we struck both breakpoints one after another (110 and 111). That proves that the previous one was set correctly.
That does not replicate in espresso though. During assertion, Child.isActive is always false (and always hits only 111 during setting child active). Just as if there were 2 separate databases used - one during action and other during assertion or something wild like that. Returning to the previous activity isn't an issue this time, as I tried removing it at all and it didn't help.
Apart from that everything seems to work fine and the rest of the tests are going through.