Bug 8976 adds the ability to change the default order of the subfields in MARC framework. But it does not preserve the Repeatable or Mandatory attribute. To test : in a MARC framework, define a subfield as Mandatory. Move the subfield to an other position within the field. The Mandatory flag for that subfield is not preserved. Instead, the subfield occupying the same position is flaged as Mandatory. M. Saby
I can replicate the issue. To test: - Edit subfields of any field - Mark the last subfield in the row as mandatory - Save - Edit again and verify the field shows as mandatory - Move the field to the first position and save - Verify the original field is no longer mandatory, but the last subfield is now Updating severity as this is a data issue.
Created attachment 137874 [details] [review] Bug 29001: Fix framework edition when subfields are reordered There is a flaw when subfields are ordered, the inputs are not retrieved correctly. We should not rely on the order but use an id instead. Test plan: Create, edit subfields Modify values from the different subfields Confirm that values are correctly saved
The patch fixes the issue but brings another one, I can't mark a subfield as repeatable now (if i check the 3 attributes and save the subfield will be mandatory and important but not repeatable). I don't have this issue with the current master. I change the status to failed QA, not sure it's the right one, feel free to change it.
Created attachment 137893 [details] [review] Bug 29001: Fix framework edition when subfields are reordered There is a flaw when subfields are ordered, the inputs are not retrieved correctly. We should not rely on the order but use an id instead. Test plan: Create, edit subfields Modify values from the different subfields Confirm that values are correctly saved
(In reply to Thibault Keromnès from comment #3) > The patch fixes the issue but brings another one, I can't mark a subfield as > repeatable now (if i check the 3 attributes and save the subfield will be > mandatory and important but not repeatable). > I don't have this issue with the current master. > > I change the status to failed QA, not sure it's the right one, feel free to > change it. Indeed, thanks! Should be fixed now. Yes, Failed QA is the correct status when the patch does not work as expected.
Created attachment 137904 [details] [review] Bug 29001: Fix framework edition when subfields are reordered There is a flaw when subfields are ordered, the inputs are not retrieved correctly. We should not rely on the order but use an id instead. Test plan: Create, edit subfields Modify values from the different subfields Confirm that values are correctly saved Signed-off-by: Thibault Kero <thibault.keromnes@univ-paris8.fr>
Created attachment 137991 [details] [review] Bug 29001: Fix framework edition when subfields are reordered There is a flaw when subfields are ordered, the inputs are not retrieved correctly. We should not rely on the order but use an id instead. Test plan: Create, edit subfields Modify values from the different subfields Confirm that values are correctly saved Signed-off-by: Thibault Kero <thibault.keromnes@univ-paris8.fr> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Created attachment 137992 [details] [review] Bug 29001: (QA follow-up) Remove code duplication Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.05
lots of conflicts on the templates when trying to apply to 21.11.x. Please provide a backport patch if needed.
(In reply to Lucas Gass from comment #10) > Backported to 22.05.x for 22.05.05 Bug 31238 is also needed in 22.05 because this 29001 brings a bug and fixed in this way: - my $authorised_value = $input->param("authorised_values_$id"); + my $authorised_value = $input->param("authorised_value_$id"); in 31238.