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

feat(cost center): add link to edit for bases #5976

Conversation

jniles
Copy link
Contributor

@jniles jniles commented Oct 5, 2021

Adds a link to the cost center allocation basis edit modal.

Closes #5957.

@jniles jniles requested a review from mbayopanda October 5, 2021 09:43
@jniles jniles force-pushed the 5957-create-or-edit-allocation-keys-by-cost-center branch from 6eedb40 to b92abb7 Compare October 5, 2021 11:39
@jniles
Copy link
Contributor Author

jniles commented Oct 5, 2021

@mbayopanda, I've fixed all the bugs in this. Can you give me a review?

@jniles jniles force-pushed the 5957-create-or-edit-allocation-keys-by-cost-center branch from b92abb7 to 3f7c463 Compare October 5, 2021 11:41
@jmcameron jmcameron self-requested a review October 5, 2021 12:59
Copy link
Collaborator

@jmcameron jmcameron left a comment

Choose a reason for hiding this comment

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

I looked this over. It looks good to me. I tested this with bhima_test and the sample data and ti seems to work well.

However, I found one thing disconcerting: When you use the new "Edit allocation basis" for one of the cost centers, it does bring up the right edit modal. However when you are finished with it, it leaves you on the cost_center/allocation_bases (Allocation Bases) page. It seems to me that it should put you back on the Cost center registry page.

Adds a link to the cost center allocation basis edit modal.

Closes IMA-WorldHealth#5957.
@jniles jniles force-pushed the 5957-create-or-edit-allocation-keys-by-cost-center branch from 3f7c463 to 247185e Compare October 5, 2021 14:16
@jniles
Copy link
Contributor Author

jniles commented Oct 5, 2021

@jmcameron this is a good suggestion. I'll take a whack at it, but I have a feeling it will require a fair amount of work. If it isn't an easy fix, I'll make an issue about it for later.

@jmcameron
Copy link
Collaborator

@jmcameron this is a good suggestion. I'll take a whack at it, but I have a feeling it will require a fair amount of work. If it isn't an easy fix, I'll make an issue about it for later.

Given the time crunch, just create an issue and release the PR. We can fix that later.

-Jonathan

Adds a better UX by navigating back to the $state the user came from
after completing the allocation basis form.
@jniles
Copy link
Contributor Author

jniles commented Oct 5, 2021

I think I got this working. Test again?

@jmcameron
Copy link
Collaborator

Works great!

@jmcameron jmcameron self-requested a review October 5, 2021 14:44
Copy link
Collaborator

@jmcameron jmcameron left a comment

Choose a reason for hiding this comment

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

Looks good and works fine.
Nice fix with the cancel and submit going back to the Cost Center page. That seems more natural to me.

@jmcameron
Copy link
Collaborator

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 5, 2021

Build succeeded:

@bors bors bot merged commit 3ad1e8c into IMA-WorldHealth:master Oct 5, 2021
@jniles jniles deleted the 5957-create-or-edit-allocation-keys-by-cost-center branch October 5, 2021 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Directly link allocation key(s) from cost center
2 participants