Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: add support for finger unlock activities #186

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

sfelf
Copy link
Contributor

@sfelf sfelf commented Sep 23, 2024

Closes #185

"id": "e05b1c22-71d1-44b8-8a4c-e729174a8972",
"timestamp": 1726770544000,
"icon": "https://d33mytkkohwnk6.cloudfront.net/app/ActivityFeedIcons/finger_unlock@3x.png",
"action": "finger_unlock",
Copy link
Owner

Choose a reason for hiding this comment

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

Is it possible to lock with finger as well?

ie is there a finger_lock?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch! I wasn't expecting there would be, as there isn't a pin_lock action defined in the code. However, after testing, I discovered that there are finger_lock and pin_lock actions. I have added support for the finger_lock action in this PR, and I will open another PR to add the pin_lock action.

@@ -137,6 +139,7 @@
ACTION_LOCK_ONETOUCHLOCK_2,
ACTION_LOCK_PIN_UNLATCH,
ACTION_LOCK_PIN_UNLOCK,
ACTION_LOCK_FINGER_UNLOCK,
}
REMOTE_ACTIONS = {
Copy link
Owner

Choose a reason for hiding this comment

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

ACTIVITY_TO_FIRST_LAST_NAME needs to be added for cases where the user isn't sent

Copy link
Contributor Author

@sfelf sfelf Sep 23, 2024

Choose a reason for hiding this comment

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

I didn't add these new actions to ACTIVITY_TO_FIRST_LAST_NAME because I didn't see ACTION_LOCK_PIN_UNLOCK added, and the functionality of the actions seemed the same. Is it necessary to add these actions to the dictionary? If so, do we also need to add the ACTION_LOCK_PIN_* actions to the dictionary?

Copy link
Owner

@bdraco bdraco Sep 23, 2024

Choose a reason for hiding this comment

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

Yes there are rare cases where the api doesn't return a user and we need a fallback. The Pin unlocks will probably break in this case unless we add them (or show an empty string)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I have added the fingerprint actions to ACTIVITY_TO_FIRST_LAST_NAME map. I will create a separate PR to add the pin actions as well.

Copy link

codecov bot commented Sep 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.27%. Comparing base (4453d24) to head (3ee93ab).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #186      +/-   ##
==========================================
- Coverage   72.48%   72.27%   -0.21%     
==========================================
  Files          31       31              
  Lines        2293     2294       +1     
  Branches      396      378      -18     
==========================================
- Hits         1662     1658       -4     
- Misses        569      572       +3     
- Partials       62       64       +2     
Flag Coverage Δ
72.27% <100.00%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bdraco bdraco merged commit fb5ef13 into bdraco:main Sep 23, 2024
7 of 8 checks passed
@bdraco
Copy link
Owner

bdraco commented Sep 23, 2024

Thanks @sfelf

@sfelf sfelf deleted the finger_unlock branch September 23, 2024 23:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for finger_unlock action in the /houses/<house_id>/activities API response
2 participants