change name of "currentFacilityId" in core state

[thanksameeelian]

Observed behavior

currentFacilityId is a getter present in core state. without prior knowledge of the getter's specifics, it's easy to misunderstand what it represents based on its name.

currentFacilityId directly gets facility_id from state, so it reflects the current user's facility id, aka their default facility if they are a multi-facility user. currentFacilityId has no relation to the facility you're currently browsing if you're a multi-facility user looking through an imported facility's classes & details in Coach & Facility - it will always reflect that user's default facility.

recently, it was discovered that using currentFacilityId as a fallback if a facilityId prop was not provided has caused routing problems in Coach & Facility for multi-facility users. it's unclear whether using currentFacilityId as a fallback was the result of a misunderstanding of currentFacilityId or if the multi-facility use-case was not top of mind when the routing code was written, but the lack of clarity around the name did contribute to more time being needed to identify the problem while debugging.

Expected behavior

proposal: rename currentFacilityId to userFacilityId (or another name that does more to clarify what the getter is retrieving)

User-facing consequences

before the issues were addressed, use of currentFacilityId as a fallback during routing behavior contributed directly to bugs like #10430 & #10673. it's not clear if the name of the getter itself caused it to be misused - it may be that the multi-facility use case was simply not accounted for - but it did contribute to it taking a little longer for me to identify and resolve the problems, as its meaning was unclear and its use was unexpected.

[Ghat0tkach]

Hello , Im very new to the Kolibri project and would like to start my contributing journey with this issue , May I be assigned for this one?

[MisRob]

Hi @Ghat0tkach, thank you for your interest. You're welcome to take this issue on. If you haven't done so already, please have a look at our developer documentation. This issue is to be resolved on the develop branch so target your pull request there. As part of your pull request, you're welcome to add yourself to the list of contributors in AUTHORS.md file.

[Ghat0tkach]

@MisRob thank you for the warm welcome , I do apologise I am not familiar with the code base but upon exploring a bit
am i supposed to change the below currentfacultyID @thanksameeelian

Thank you

[thanksameeelian]

hi @Ghat0tkach, thanks so much for diving in!

i think once we've pointed you in the right direction, this issue should be relatively straightforward but will require some attention to detail 🙂

currentFacilityId is referenced in a few places in kolibri and is initially defined here in session.js. from searching the repo, i found around 11 additional files that contain references to currentFacilityId (including the one you mentioned in your comment, so you're definitely on the right track) which will need to be changed, though i'd encourage you to search as well.

we'd like to find every usage of currentFacilityId and be sure they're all renamed to userFacilityId so that the true meaning of the getter is more self-evident.

i think renaming this getter will reduce confusion in the long run, so we just need to be a bit meticulous right now to be sure each file & test where currentFacilityId is referenced has been updated.

please let us know if more clarification would be helpful!

[Ghat0tkach]

@thanksameeelian Thanks a lot for assisting me in finding the original getter and all their respective references.
Will dive into code much deeper and shall change all their instances respectively.

Thanks again!