Repository navigation
Conversation
|
I just realized this isn't complete or might be not possible, because this only messes with set fields, and switching also changes problem fields. Making a draft for now as I think about what to do with the problem fields. |
b84fed6 to
5976371
Compare
|
This now shows/hides the problem fields based on set type as well, but the javascript would be a bit too complicated to update the set numbering to change to/from JITAR sets, so in that case a warning is shown to let the user they need to "Save Changes" to update the problems when changing to/from a JITAR set. |
81a48f1 to
65e84bf
Compare
|
This is probably beyond the scope of this PR, but how about adding the ability to choose an assignment type when creating a set? |
|
Since I cannot think of a good way to deal with to/from jitar sets and updating problems to the jitar naming/nesting buttons, this is now ready for review again. It does update problem fields, and just warns users when they need to "Save Changes" to move to/from jitar sets to fully update the problems to/from the jitar naming/nesting ability. |
|
@dlglin that could be a possibility, but there are multiple places a set can be created. Adding that option to the "Create" action tab in the "Sets Manager" would be doable. Though should this also be added when creating a set from the "Library Browser". Unsure if there is any other way a set can be created (outside of importing which already contains the type). |
65e84bf to
07c3385
Compare
drgrice1
left a comment
There was a problem hiding this comment.
Here are some things I see at this point.
This pull request makes me a bit wary. It seems that there might be some dangerous things in this. Particularly when it comes to changing to or from a JITAR set, but there might be things missed for the other changes as well. How versatile and safe is the code for handling the set type change and other edits to settings that are made before a save occurs?
|
I think jitar sets are currently broken in develop. Create a jitar set, create some nesting, "save" that change. When the page is reloaded, the nesting is not shown anymore. But if you then try to move some nesting around there is some weird effects, and some of the problem fields are shown that shouldn't be. So it is like the nesting is kinda there but not fully. I ran into that testing this and discovered it wasn't due to any of my changes. |
|
Currently save will update any provided inputs, or use a default value if the setting is empty. One of the things I ran into was some of the default test settings were actually set during the 'Save' to switch to the test, so I also made sure to set the default values in the form initially if they were undefined in the database. With this setup all the hidden inputs are also sent as part of the request. This does mean if you change a setting, then change the type so that setting is no longer visible. That change will get sent and be updated for the hidden fields. I didn't think that was an issue, but I could go a step further and disable all inputs that are hidden, so only the visible inputs are sent as part of the post request. In testing this more, switching to and from a jitar set will loose any changes to the problem order/nesting. I added that info to the message, and also no longer include the jitar problem fields for non jitar sets. Since switching to a jitar set would require a save anyways, and it makes no point to show them (which they would all be hidden anyways since there is no nesting). |
07c3385 to
bbe3306
Compare
I am not seeing any issues on the develop branch. I created a JITAR set and created nesting. When the page reloads the nesting is collapsed by default, but can be expanded. Moving things around seems to be fine. I am not seeing any weird effects or problem fields shown that shouldn't be (or not shown that should be). |
|
Can you clarify the issues with the JITAR sets that you are seeing? |
bbe3306 to
a562cc4
Compare
|
Goto an existing non jitar set and edit its settings. Change it to a jitar set, click "save". Got to problem 2 and 3 and indent them one, so the set now has problems 1.1 and 1.2, click "save". After that when you go look at the problems, the nesting is gone, and problem "1.1" and "1.2" are not shown. But problem "1" shows the field attempt threashold for children, so it is like it knows it has children that are not being shown. At this point if you try to nest problem "2", it then nests the two problems below it making a "1.1", "1.2", and "1.3" appear. |
|
Oh wait, it is my confusion, I didn't realize that nesting was collapsed by default. Though it does make things act a little weird if you try to nest things again. |
|
Though I see the behavior, nesting the next problem then both expands the nesting and adds that problem to it. So it is working as expected, just not how "I" expected it to. |
a562cc4 to
de7e514
Compare
|
I asked claude to see if there is any potential issues in my changes with the save logic causing problems. Claude found no other issues than the ones I have already mentioned, and that is changes to fields that are hidden will be updated, but not used since they don't affect the new assignment type. Outside of any changes to the problem order being lost with switching to/from a jitar set, there isn't anything that Claude was able to spot as dangerous. Claude did find one bug with switching to a jitar set, but that exists before my changes, |
95a4659 to
58f30a7
Compare
drgrice1
left a comment
There was a problem hiding this comment.
I am going to go ahead and approve this for now. I am not sold on the behavior with switching to and from JITAR sets though.
pstaabp
left a comment
There was a problem hiding this comment.
I think this working as well as it can--it's a bit awkward with JITAR sets, but I think it's a compromise.
|
I think I thought JITAR was broken as @somiaj did above. I don't usually use JITAR, so haven't noticed this until now. It would be helpful to have something more prominent in the UI indicating that there are children of a given problem. I think now there is just the presence of a right chevron if there are children. And I just noticed the presence of another row in the table: Attempt Threshold for Children. I missed those and thought I had lost problems. |
|
@pstaabp The UI is already over complicated on this page, and JITAR is probably rarely used, so not worth the effort IMO. I say leave it alone and just let user's get use to it if they use JITAR. Once I knew what to expect (I just expected sub-problems to be expanded by default, not collapsed), it wasn't an issue. It was just a learning curve, and I don't know of any clear way to improve it. |
When selecting the `assignment_type` on the ProblemSetDetails page, use JavaScript to show/hide set fields for the appropriate assignment type. This way one can change a set's assignment type and edit the specific assignment type fields without having to save the changes first to see the fields for that type.
58f30a7 to
f09af1e
Compare
When selecting the
assignment_typeon the ProblemSetDetails page, use JavaScript to show/hide set fields for the appropriate assignment type. This way one can change a set's assignment type and edit the specific assignment type fields without having to save the changes first to see the fields for that type.