Unify Rubric Configuration Modal #7952
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jniles
force-pushed
the
refactor-unify-rubric-configuration
branch
from
91bdb83 to
682c780
Compare
jniles
force-pushed
the
refactor-payroll-management
branch
from
5bd908f to
cd1a9d8
Compare
jniles
force-pushed
the
refactor-unify-rubric-configuration
branch
from
682c780 to
7d2bf75
Compare
jniles
force-pushed
the
refactor-payroll-management
branch
from
cd1a9d8 to
a10945c
Compare
Unifies the rubric configuration modal to configure both the label and checkboxes for applied rubrics at the same time for a more streamlined configuration.
Migrates the rubric configuration server controllers to use the `payroll/` prefix in routes. It also removes the `/setting` suffix from the routes, opting instead to have an API similar to the cash payments or invoices API where the items are provided as an array in the main payment configuration object. The next step will be to unify the client UI to allow simultaneous configuration for the payroll rubrics.
Uses a unified interface for the rubric configuration service.
jniles
force-pushed
the
refactor-unify-rubric-configuration
branch
2 times, most recently
from
83e610a to
c76496d
Compare
|
@jmcameron, I'm pretty sure this is finally able to be reviewed when you get a chance! EDIT: Finally got the end to end tests stable here. |
Fixes a race condition that caused buggy behavior in the Tax IPR Config Modal controller. Now, the submit button is disabled until all the data is available to use.
jniles
force-pushed
the
refactor-unify-rubric-configuration
branch
from
c76496d to
9bbafd3
Compare
Removes extraneous file from the repository.
Does not crash when the rubric items are empty.
jniles
force-pushed
the
refactor-unify-rubric-configuration
branch
from
5167db1 to
5cd0d39
Compare
Block modal submission when required fields are not filled in and ensure that `unset` tests work to allow creating a modal with empty rubric fields.
jniles
force-pushed
the
refactor-unify-rubric-configuration
branch
from
895d222 to
7aa1945
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal of this PR is to reduce complexity, both for users and developers. It accomplishes this by removing unnecessary steps for the user, and by namespacing and reducing the amount of code for developers.
Update: it also fixes a bug in the IPR Config client code. See the "Bug Fix" section below.
Specifically, the rubric configuration modal UI and API have been updated to remove separate routes and interfaces for the
/settingroutes that configuredrubric_config_items. This creates a single modal for the user to manage. On the developer side, it namespaces the API route to behind thepayroll/prefix and updates the data model in transit to makerubric_config_items a property of therubric_configoption, similar to how invoices, vouchers, and cash payments work with their associated_items.Here is the updated User Interface.
In the future, we might use the
bh-checkbox-treefor the rubrics, but that relies on having a saneclassify()function for the different types of rubrics. At the moment, the rubric classification is not well enough understood by me to make those kinds of changes.Bug Fix: IPR Config
In addition to the refactoring of the Rubric Configuration interface, I have also fixed a race condition in the IPR tax configuration modal. In some cases the
vm.taxIprIdwas not defined, which could lead to undefined behavior. I added the$qmodule to do the following:Additionally, I also refactored the URL handling to remove the duplicate
configurationtext in the URL. Previously, the URL for editing a configuration was/ipr_tax/configuration/1/configuration/4/edit. Now, it is simply/ipr_tax/configuration/1/edit/4, which more closely follows standard REST conventions ofnamespace/{id}than previously.