Skip to content

Feature kfs slingshot.ccb.update#84

Open
guymaness2 wants to merge 7 commits into
SparkDevNetwork:masterfrom
KingdomFirst:feature-kfs-Slingshot.CCB.Update
Open

Feature kfs slingshot.ccb.update#84
guymaness2 wants to merge 7 commits into
SparkDevNetwork:masterfrom
KingdomFirst:feature-kfs-Slingshot.CCB.Update

Conversation

@guymaness2

Copy link
Copy Markdown

Changes:

  • Added hashing to compound Id generations to ensure length of Ids do not exceed int limitations.
  • Added option to export tool to enable/disable breaking Directors out into parent groups.

@croske

croske commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Hello, and thank you for your request.
We can see the intention behind the hash for the imported Id, but think that might be a limiting addition, especially when tracking the original values.
Regarding "Directors", is there CCB documentation we can review about this hierarchy in the system?

@guymaness2

Copy link
Copy Markdown
Author

Hello, and thank you for your request. We can see the intention behind the hash for the imported Id, but think that might be a limiting addition, especially when tracking the original values. Regarding "Directors", is there CCB documentation we can review about this hierarchy in the system?

We resorted to hashing because the existing method of generating ids was causing unavoidable errors while extracting data from a legitimate CCB account. The compound numbers were growing beyond the boundaries of valid integer types. I would welcome alternative approaches. Just know this was not a "straw man" fringe case. We had to make a change to get it to work with a real-work use case.

As for the "Directors", I found some general documentation on the group structure that mentions them. The original slingshot code arbitrarily creates a parent group for every Director, then adds the child groups under each "director" group. This forced hierarchy transformation during extraction was undesirable in some cases. The added code presents the choice as a configuration option in the configuration screen, defaulting to "true" to match previous behavior.

Thank you for reviewing this request.

@guymaness2

Copy link
Copy Markdown
Author

Hello, and thank you for your request. We can see the intention behind the hash for the imported Id, but think that might be a limiting addition, especially when tracking the original values. Regarding "Directors", is there CCB documentation we can review about this hierarchy in the system?

We resorted to hashing because the existing method of generating ids was causing unavoidable errors while extracting data from a legitimate CCB account. The compound numbers were growing beyond the boundaries of valid integer types. I would welcome alternative approaches. Just know this was not a "straw man" fringe case. We had to make a change to get it to work with a real-work use case.

As for the "Directors", I found some general documentation on the group structure that mentions them. The original slingshot code arbitrarily creates a parent group for every Director, then adds the child groups under each "director" group. This forced hierarchy transformation during extraction was undesirable in some cases. The added code presents the choice as a configuration option in the configuration screen, defaulting to "true" to match previous behavior.

Thank you for reviewing this request.

I'm sorry, after re-reading what I wrote, and looking at the code changes, I need to re-state something I wrote with regard to the hashing. The original issue we were encountering is that the code was using a simple structure of "9998 + Director Id" as a "unique" group id. The problem is that the same director could be assigned in multiple departments, resulting in the same Id. This was fixed by introducing the structure of "9998 + Director Id + Department Id" to ensure unique director groups. Once this change was implemented, we started encountering the issue of the number being too large, which is when we converted to a hash.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants