Closed bingzhang closed 1 year ago
Hi, @bingzhang, I see that there is an existing code where we check whether the incoming category is known. I think it may be better to modify that with a continue statement to ignore unknown/excluded categories. That way, we do not have to update included and excluded categories during future changes. Also, please see my comment on this issue: https://github.com/rokwire/events-manager/issues/1085#issuecomment-1574200638. Thanks.
Hi, @bingzhang, I see that there is an existing code where we check whether the incoming category is known. I think it may be better to modify that with a continue statement to ignore unknown/excluded categories. That way, we do not have to update included and excluded categories during future changes. Also, please see my comment on this issue: #1085 (comment). Thanks.
update the code. please check again.
@sandeep-ps please review again. thanks.
Wouldn't constants.py eventTypeMap need to be changed? I see "Community/Service" and "Ceremony/Service" still in line https://github.com/rokwire/events-manager/blob/ef2a95c21d0fe8de64b0f75cb27f6088980a33b3/utilities/constants.py#L51 Or maybe I didnt understand the issue correctly?
Wouldn't constants.py eventTypeMap need to be changed? I see "Community/Service" and "Ceremony/Service" still in line
Or maybe I didnt understand the issue correctly?
I will jump in here. It's removed in a different PR that deals with the mapping. https://github.com/rokwire/events-manager/pull/1088/files. In the issue, I mentioned that the new mapping might just be needed, but we saw that some logic update is needed for the categories to be excluded.
Description
Please provide a summary of the pull request and the issue it fixes. Please add necessary details, context, dependencies, explanation of when review is needed (see next section), etc.
Fixes #(add issue number here and remove parentheses)
Review Time Estimate
Please give your idea of how soon this pull request needs to be reviewed by selecting one of the options below. This can be based on the criticality of the issue at hand and/or other relevant factors.
Type of changes
Please select a relevant option:
Checklist:
Please select all applicable options: