Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: ✨ allow for collection of multiple trees per day #18

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: ✨ allow for collection of multiple trees per day #18

wants to merge 2 commits into from

Conversation

DontBlameMe99
Copy link

Modify the code to allow for multiple trees per day.

This commit also changes the display of the trees per day in the Stats -> Details view

New version of the details page (After the change

new

Old version of the details page (Before the change

old

What has changed
  • Modified SQL (in DayDao.kt) statement to use steps / goal instead of just counting the days where the steps >= goal
  • Modified other (StatsSummary.kt, StatsDetailsViewModel.kt) code locations to use steps / goal instead of just using steps >= goal
  • Changed the display of the daily tree goal from "just achieved" to "x trees collected" (see old vs new version of the details page)
  • Removed/Refactored (now) unnecessary things in the strings.xml- Changed the StatsDetailsViewModel.kt from a simple boolean if the tree has been collected to an int which indicates how many trees were collected on that day
  • Changed the StatsDetailsState.kt from a simple boolean if the tree has been collected to an int which indicates how many trees were collected on that day

closed #8

feat: ✨ allow for collection of multiple trees per day
03e10d3 
Modify the code to allow for multiple trees per day

closed bk20dev#8
@bk20dev
Copy link
Owner

bk20dev commented Jul 11, 2024

And by the way, don't hesitate to create multiple commits. It's generally a bad idea to force push them to origin.
If you want to fix something in the commit you created and not yet pushed, you can use git commit --amend (read more how to use it).

@DontBlameMe99
Copy link
Author

And by the way, don't hesitate to create multiple commits. It's generally a bad idea to force push them to origin. If you want to fix something in the commit you created and not yet pushed, you can use git commit --amend (read more how to use it).

Perfect that I read that just AFTER creating that commit ._. Yeah, thanks for letting me know. I love to learn and clearly you have more experience than me! ❤️

bk20dev
bk20dev requested changes Jul 14, 2024
View reviewed changes
app/src/main/java/pl/bk20/forest/core/data/source/DayDao.kt
@@ -9,7 +9,7 @@ import java.time.LocalDate
@Dao
interface DayDao {

@Query("SELECT steps / goal as treeAmount FROM day WHERE steps >= goal")
@Query("SELECT SUM(steps/goal) FROM day WHERE steps >= goal")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can drop the WHERE steps >= goal. The SUM will only count collected trees. This is because we're dividing integers here.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least I think so. Please confirm it still works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bk20dev bk20dev bk20dev requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

More trees
2 participants
@DontBlameMe99 @bk20dev