Skip to content
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

apply the view costCenter filter to edit #696

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

floatingatoll
Copy link
Contributor

Something in the CIS pipeline is appending ".0" to cost centers, which is then displayed to user only when editing their profiles, not when viewing those of others.

This patch applies the logic from the view page to the (readonly) field on the edit page as well, so that we're internally consistent within Dinopark and also consistent with the current 2023-era cost centers as shown in Finance documents.

We still need to find what's injecting .0 and fix that, but this removes the user-visible impact for Dinopark until we do so, and will noop once that's removed upstream.

@floatingatoll floatingatoll requested a review from gcoxmoz February 2, 2023 18:46
@floatingatoll floatingatoll self-assigned this Feb 2, 2023
Copy link

@gcoxmoz gcoxmoz left a comment

Choose a reason for hiding this comment

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

Hrm.
I do see the described issue when I edit my profile. I do see this as scoped to the right area of the codebase.
I don't know the language. I don't know the strong-type-iness in the language. I don't know that it's a string, like, maybe it's presenting as a float and maybe this is going to blow up.

This is a pro-forma approval ala "I have no better offer and it looks like it makes sense, let's find out" If it blows up, it wouldn't surprise me. But, it's low-stakes and no big deal on a revert, so let's go.

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