Starred Issues

[brericha]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization

Description

Implements a starred issue feature. Adds an icon to the app bar in IssueActivity that toggles a starred state. This state is stored in a new database table that is fetched during onResume in MainActivity and "trimmed" when the issues list is received from the API.

Another notable change, I removed the app bar title from IssueActivity as it was already being truncated on my phone and adding another icon just further shortened it. Considering the text that was there (the issue title) is directly below I felt that having the first Xteen whatever characters of it in the app bar seemed silly.

• Closes Create a way for users to "star" their favorite issues and filter by their starred issues #234

Were the changes tested?

• Yes, automated tests in DatabaseHelperTest.java
• Yes, manually tested: Toggling issues as starred/unstarred and checking that changes reflect in main issues list

[dektar]

Thanks for working on this! I agree with the way you've implemented the behavior -- I like leaving the issue title off the issue activity and putting the star in there again. I like the drawable on the right that pushes over issue title only when needed, but the contrast isn't sufficient right now (more below).

I have some high-level comments about how we merge starred issues within issues adapter, and about not deleting old starred issues. I will take a more detailed look after these are addressed.

I'd love if we could add some integration tests for showing/hiding the starred issues in the filter. however, we don't have any filtering tests like this yet anyway so if it's not feasible then we can wait until I get those set up.

Thanks again for working on this!

[dektar]

We should be able to add the star as a svg without having to add all these png assets in different directories -- it'll keep the app binary size from increasing as much and reduce overhead for us if we change it in the future. You can download the star from here: https://fonts.google.com/icons?selected=Material+Symbols+Outlined:star:FILL@0;wght@400;GRAD@0;opsz@24&icon.size=24&icon.color=%23e8eaed

To fill it I think you can make an XML using that SVG and then set the android:fillColor.

[dektar]

This icon isn't high enough contrast. Can we make the star one of the app colors, like dark blue or red or bright blue instead?

[dektar]

In the DBHelper file, I suggested:

Can we instead say getStarredIssues(List ids) and have the DB lookup just look through the starred ones?

I suggest that getting starred issues from an existing list might also be handy because it means we won't have to call notifyDataSetChanged, instead we can just update our mIssues and notifyItemChanged for each issue that is starred.

In other words, this class will do a lot less work and we'll won't need to redraw the issues list a second time.

[dektar]

If we annotate mIssue with isStarred you wouldn't need to set/get this in the state.

[dektar]

please add a test that there's no issue if we remove an issue that's not in in the starred issues DB.

[dektar]

please add a test that adding the same issue twice doesn't cause trouble.