Create Object tool on Main Menu

[crhallberg]

TODO

• Flesh out post route on API
• Add tests
• Fix broken functionality
• Fix broken tests (following latest dev merge)
• Decide how to handle and display feedback after object creation completes

[demiankatz]

@crhallberg, I spent a bit of time today playing with this -- it seemed like a good excuse to practice some of the techniques @diboy2 has been using elsewhere. I modernized the object editor code and rewrote things so that React controls the form instead of having it behave like a standard browser form (which I don't think is what we would want!).

I'm seeing some warnings about DOM manipulation which I think may be related to the library you're using for the selectable tree; I wonder if that's not fully compatible with React.

I also haven't had time to do anything on the server side yet. I was hoping to work on that today, but all the React modification distracted me. :-)

[crhallberg]

Not sure about the DOM warnings, I used the TreeView from Material UI, which was built for React. https://material-ui.com/es/api/tree-view/

[crhallberg]

There is a recent update, apparently. https://twitter.com/MaterialUI/status/1438518915236126723

[demiankatz]

@crhallberg, doesn't look like the latest release impacts the lab, where the Tree-related components still seem to be stuck. And in case it helps, here's the exact error I'm seeing:

Warning: findDOMNode is deprecated in StrictMode. findDOMNode was passed an instance of Transition which is inside StrictMode. Instead, add a ref directly to the element you want to reference. Learn more about using refs safely here: https://fb.me/react-strict-mode-find-node
    in ul (created by Transition)
    in Transition (created by ForwardRef(Collapse))
    in ForwardRef(Collapse) (created by WithStyles(ForwardRef(Collapse)))
    in WithStyles(ForwardRef(Collapse)) (created by ForwardRef(TreeItem))
    in li (created by ForwardRef(TreeItem))
    in ForwardRef(TreeItem) (created by WithStyles(ForwardRef(TreeItem)))
    in WithStyles(ForwardRef(TreeItem)) (at CreateObject.jsx:79)
    in ul (created by ForwardRef(TreeView))
    in ForwardRef(TreeView) (created by WithStyles(ForwardRef(TreeView)))
    in WithStyles(ForwardRef(TreeView)) (at CreateObject.jsx:119)
    in form (at CreateObject.jsx:101)
    in CreateObject (at Routes.jsx:35)
    in Route (at Routes.jsx:34)
    in Switch (at Routes.jsx:18)
    in Routes (at VuDLPrep.jsx:15)
    in Router (created by BrowserRouter)
    in BrowserRouter (at VuDLPrep.jsx:14)
    in div (at VuDLPrep.jsx:10)
    in VuDLPrep (at src/index.js:14)
    in StrictMode (at src/index.js:13) index.js:1

There's a long discussion about the issue at mui/material-ui#13394 and it sounds like it has been fixed in release 5.0... but based on what I'm seeing it seems that perhaps the fix doesn't extend to the lab components (yet?).

[demiankatz]

@crhallberg, a bit more investigation added greater clarity to this situation: Material-UI has renamed itself to MUI, and the package names have changed. So we probably need to replace @material-ui/lab with @mui/lab to get a newer version. However, @mui/lab requires a higher version of React than we are currently using. Should we upgrade? How big of a project is that likely to be? Something we should perhaps discuss at today's meeting. Maybe an upgrade will be fairly straightforward when @diboy2's refactoring work is complete.

[demiankatz]

I've made some more progress on this, and object creation now actually works! There are a couple of open issues, though:

• For some reason, even though successful object creation returns a 200 status, the React component is treating this as an error; I haven't figured out why yet.
• We should support the creation of objects with no parents (since we need a way to create the first object in the repository, after all!). That may require some extra thought in terms of UI and validation.

[demiankatz]

I've made more progress on this -- the control now supports specifying no parent PID, as well as locking the control to a hard-coded parent. I think this should cover all the major use cases.

We definitely need to add tests, because with the properties I've just added, there are quite a few scenarios that need to be checked. I haven't tested them all yet -- just enough to ensure that the basic functionality works.

The problem I noted above with the React component treating 200 success responses as errors is still occurring. I haven't dug into it yet because I suspect we need a totally different mechanism for providing feedback anyway. Perhaps this component should throw an event that can be hooked, since in different contexts, I suspect we'll want to respond to object creation in different ways.

[demiankatz]

Thanks again for the review, @diboy2 -- I have improved and simplified the code according to your suggestions. There is still some work outstanding regarding processing of responses, but I'm going to wait until you make more progress on your AjaxHelper refactoring -- I suspect that this branch can be further improved after that work is done, so I'm going to hold off on further changes for now.

[demiankatz]

Just an update to note that I have begun working on updating this to use the latest code from dev, but at the moment, it is completely broken. I've pushed my changes anyway since I don't want to lose them, but I have run out of time for today. I'll try to get this back to a working state when time permits!

[demiankatz]

I've made some progress on restoring functionality following the last merge; I believe that the control is now working correctly again. However, I'm having trouble figuring out how to mock the AJAX behavior using the new mechanisms in order to get the tests working again. Perhaps @diboy2 can provide some advice when time permits!

[demiankatz]

With some help from @diboy2, I managed to get everything working again. Hooray!

[demiankatz]

I've made significant progress here, integrating the create object tool with the object editor. There are still some outstanding issues, but it may be worth considering a merge of this work-in-progress so that we can focus on other details separately. Here are the key problems:

1.) The user interface is not elegant and needs to be improved -- I was simply aiming for functionality for now.

2.) There is no accounting for ordered collections yet, so you can't create an object at a specific position inside an ordered collection. That needs to be added, but it's complicated! Trying to get through the basics first.

3.) When wiring everything up with the Camel toolbox, objects created through this tool do not get indexed correctly. I think there may be a bug in the logic to prevent duplicate indexing jobs, but I'm not sure; it will require further troubleshooting. However, that problem is not directly related to the functionality here, and I do not think it should block merging this. I have also opened #94 to make the queue management code better-organized, which I believe will help with investigating this problem.

Perhaps we can discuss this in more detail at Thursday's meeting.

[demiankatz]

In my previous comment, I mentioned some indexing problems, which turned out to be cache-related. #95 fixes these problems and also reduces the number of errors displayed in the console during indexing (by being more tolerant of missing Dublin Core).

I believe that this is now in a state that can be merged, on the understanding that further improvements to UI and functionality are needed in the long term. I believe this is a good "minimum viable product" step forward, however. Any objections?