Migrate CIDER Resource Allocation Interface to XRAS Admin and implement relative_order drag and sort functionality

[asimregmi]

Overview

This PR implements the migration of "Allocations" page from CIDER into XRAS Admin resources/resource_id page. It also adds a new dragging and sorting functionality in the main /resources listing page.

Branch ramps-583-implement-relative-order-column-in-allocations_process_resources is checked out from ramps-554-migrate-cider-resource-allocations-interface so has both changes from the two Jira tickets.

Related

• RAMPS-554
• RAMPS-583

Changes

• added EditResource.jsx and supporting React components, helpers, and reducers inside /src/edit-resource
• added Resources.jsx and supporting reducers and helper functions inside /src/resources
• added new shared components and made changes to existing shared components Grid.jsx, Alert.jsx
• See also: updated 'Save Resource' button logic in xras_admin/app/views/resources/show.html.erb
• change in app/views/resources/index.html.erb and app/views/resources/show.html.erb to accomodate new React components

Testing

1. git pull origin/ramps-583-implement-relative-order-column-in-allocations_process_resources in xras_admin repo
2. git pull origin/ramps-583-implement-relative-order-column-in-allocations_process_resources in xras-ui repo
3. Add a new relative_order integer column in xras.allocations_process_resources table
4. npm run dev in xras-ui directory

[yomatters]

@asimregmi, this is looking really good! I've made some specific suggestions about parts of the code inline. Here are a few general points of feedback, mainly about the user experience.

• On the resource list page, please remove the paragraph beginning with "Only resource submission questions can be modified...". That's not true anymore with these changes.
• On the resource edit page, I suggest renaming the "Descriptive Text" column to "Help Text." I think that will help to distinguish it from the "Allocations Description" field.
• I'd recommend changing the names of the buttons above the Allocations Types tables to "Manage Allocation Types" and "Manage Required Resources." Then in the modal, you can display a list of checkboxes instead of a select field. That will also allow users to remove an item, which I think is particularly important for the required resources.
• With Vite, you don't need to import the global React object in each component unless you're explicitly referencing it.
• I'd recommend using a code formatter and linter to format and check your JavaScript code. I like Prettier for formatting and ESLint for linting. Both have extensions that integrate them with VSCode (and probably other editors), or you can run them from the command line.

[yomatters]

This approach, where the row contains configuration objects for each field, is interesting, though it's different from what I did in the My Projects interface. Let's discuss this offline and decide how we're going to handle editable fields in the grid so that we can use the same approach everywhere.

[yomatters]

I'd prefer not to add a jQuery dependency if we can avoid it, though I realize that may be tricky with Bootstrap 2. Have you tried the Tooltip component from React Bootstrap? If the CSS classes haven't changed, it may work with Bootstrap 2 as well.

[yomatters]

If it's not too much work, it might be nice to break the drag-to-reorder logic out into a separate component. I can imagine it's something we might want to use elsewhere.

[yomatters]

@asimregmi, this is looking really good! Here are a few additional suggestions related to the latest changes. Hopefully most of these are just minor tweaks!

1. Once a discount rate is saved, its exchange_rate field should not be editable. We don't want the rate to be changed once the record is saved since that rate may already have been used in an exchange, and we want to be able to keep a history of the rates used for exchanges.
2. If possible, I think it would be better not to show the date fields at all in the base rate row. Showing them disabled implies that there are circumstances where they would be editable.
3. I'm not sure it's necessary/helpful to show the help text for exchange rates both below the table and in the tooltip. I'd probably just put it in the tooltip. It might also be good to label that column "Exchange Rate" for consistency with the help text.
4. It looks like there's a z-index issue with the tooltips. If they go below the bottom of the table, they're getting cut off.
5. "Allowed Actions" has a tooltip that seems to be a placeholder.
6. For the required resource columns, I'd start the column label with the word "Require" (e.g., Require Bridges-2 Regular Memory). I think there was some confusion about the purpose of those columns the last time you demoed this interface.
7. This is a minor thing, but it looks like the Allocation Types and Exchange Rates tables have an extra border around them compared to the Resource Submission Questions table.
8. @rebeccaeve may have a better sense than I do of best practices, but I think you may be overusing useCallback and useMemo. My understanding is that those hooks should only be used for logic that is computationally expensive, or in cases where components are re-rendering frequently. Wrapping most of the state logic in those hooks makes the code harder to read, and in many cases, I don't think it's really improving performance (e.g., when the dependency is resourceData, which changes any time the state changes).
9. You don't necessarily need to change this now, but in the future, you might consider using Redux Toolkit instead of just Redux (see src/projects/helpers/apiSlice.js for an example in this repo). It helps to cut down on boilerplate code and makes reducers much easier to write and to read.

[rebeccaeve]

Matt is correct about the usage of useCallback and useMemo. Those are used to speed up the performance of an app, and generally aren't necessary if there's not an expensive calculation or network fetch happening. Technically there's nothing wrong with using them more frequently, but if not used carefully it can actually introduce some fun logic bugs! Like when you're getting a "stale" version when you're not expecting it.

[asimregmi]

@yomatters and @rebeccaeve, thank you both for the review!
@yomatters I have made the following changes in regards to your comments:

• added new flags isStartDateInFuture canEditStartDateAndRate to enable/disable rate and date fields based on future start date and end date.
• Removed Date Fields from the base rate row.
• Remove the <div>...</div> containing the explanation notes/help text and the extra tool tip placeholder from Allowed Actions.
• Required Resources columns now populate with Require ${resourceName}
• Removed over usage of useCallback and useMemo hooks
• The extra border on the Allocation Types and Exchange Rates tables seems to be coming from the Grid component and the Questions uses the Rails template. I don't have a fix for now but since this seems to be out of scope but we can get that Questions to also use React components in the future. (this will be also help reduce some wanky logic I am using to expose the React submit button and date validation errors to rails.)
• Thanks for the Redux Toolkit suggestion for the future. I will look into it.