-
Notifications
You must be signed in to change notification settings - Fork 143
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
Fix race condition in resource-instance-select.js #10210 #10211
Conversation
If a document has >1 resource instance select widgets, they may both try to initialize select2 widgets, which manage references to the DOM and cannot handle parallel init'ing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see any issues with this change, the behaviour seems to be fine.
I have noticed however that the graph icons are no longer showing against the resources that come up in the list. I only get the fa fa-question
, no matter what I set.
I have tracked this to the following line, where the Graph objects are turned into a dict -
arches/arches/app/views/api.py
Line 359 in 9aea9d1
graph = JSONSerializer().serializeToPython(graph, sort_keys=False, exclude=["is_editable", "functions"] + exclusions) |
After this line, the iconclass property no longer has a value, so this needs to be fixed. I'll pop some screen shots below.
Thanks for flagging that, I'll take a look. |
Hey @aj-he I can't reproduce that myself, I still have icons in the UI and I when I break at the line in your screenshot, I still have a graphid: This is the value of ['cards', 'domain_connections', 'edges', 'nodegroups', 'nodes', 'widgets'] Is yours different? For me, findings were the same on this branch and on dev/7.5. |
Whoops, you called out |
I still can't reproduce, but it looks like it could be a symptom of the graph publication lacking the iconclass. I know your graph object has an iconclass, but could you verify whether its related graph publication record has an iconclass? Or just a quick retest after a republishing. |
My mistake @jacobtylerwalls. I had been playing with graphs in the database and had forgotten to repubilsh them! Sorry for wasting your time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Types of changes
Description of Change
If a document has >1 resource instance select widgets, they may both try to initialize select2 widgets, which manage references to the DOM and cannot handle parallel init'ing.
Issues Solved
Fixes #10210
Sourced to a36a946.
Checklist
Ticket Background
Testing instructions
Ensure that icons still appear in the resource instance select widget per #8305.