-
-
Notifications
You must be signed in to change notification settings - Fork 94
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.
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
🐛 goingtocamp missing sites #293
base: main
Are you sure you want to change the base?
Conversation
@@ -175,7 +175,7 @@ def get_all_campsites(self) -> List[AvailableCampsite]: | |||
# be viable for camping. Skip all zero-capacity sites. | |||
if ( | |||
not site_details["minCapacity"] | |||
or not site_details["maxCapacity"] | |||
and not site_details["maxCapacity"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's where we allow sites with a maxCapacity, but not a minCapacity, to get selected
search_filter.equipmentCategoryId = NON_GROUP_EQUIPMENT | ||
search_filter.subEquipmentCategoryId = equipment_type_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's where we only use the equipmentCategoryId
when subEquipmentCategoryId
is also used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was the bug that "group sites" were ending up in the result set?
Assuming equipment_type_id
is a non-group equipment type, the addition of the NON_GROUP_EQUIPMENT
filter seems like it should be superfluous here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. Although I have one question out of curiosity: https://github.com/juftin/camply/pull/293/files#r1313768409
This should be merged! |
3bf16ef
to
270d051
Compare
I'm not actually sure GoingToCamp is working as expected, with or without this change. For example, this command currently returns 7 sites: camply campsites --provider GoingToCamp --campground -2147483565 --start-date 2024-05-23 --end-date 2024-05-24 --rec-area 14 --notifications silent --debug However these 7 sites are all included in the Group Campsite Area, and omitting the #1-94 area which has at least 1 available. I believe there's an issue with enumerating all MapIDs / Locations to search and some areas aren't included. |
Are there any updates here? I am seeing the same when running:
Basically it is not catching all the mapIds and doesn't list available campsites |
Description
Resolves #291
This PR makes a few changes:
NON_GROUP_EQUIPMENT
in every API request. By default, no equipment type is requested.Has This Been Tested?
Yes - all snapshots needed to be recreated because we're passing different API params