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
ensure MultipleChooserPanel selecting unique objects #11855
base: main
Are you sure you want to change the base?
ensure MultipleChooserPanel selecting unique objects #11855
Conversation
Manage this branch in SquashTest this branch here: https://elhussienalmasriensure-multipl-d6g25.squash.io |
.not('.deleted') | ||
// eslint-disable-next-line func-names | ||
.each(function () { | ||
const inputValId = $(this).find('input[type="hidden"]').val(); |
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.
This is way too brittle - we can't assume that any numeric hidden input we find inside an InlinePanel child element is an object ID for the chooser we're interested in. Even if it did work, InlinePanel shouldn't be digging in to the internals of the forms it's managing to this extent.
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.
Thanks for feedback , this hidden input hold the unique ids of "MultipleChooserPanel" objects or checkboxes,
so what is your suggestions , may I put the unique ids in another element but the unique ids already in the hidden inputs ,so please tell me what must I do.
by the way I agree
we can't assume that any numeric hidden input we find inside an InlinePanel child element is an object ID for the chooser we're interested in
so may I put another unique attribute to the hidden input so its unique from the other hidden inputs.
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'd say this logic doesn't belong in InlinePanel, because there's no expectation that an InlinePanel will contain any choosers at all (for example, it can be a list of external links consisting of URL and label fields). If you put this function in MultipleChooserPanel instead, you can identify the relevant chooser widget by its id/name (opts.chooserFieldName
) and retrieve its value through the chooserWidgetFactory
object, just like we do when setting the widget's value. (Maybe it will help to add a utility method to InlinePanel for iterating over the active child forms - I see that this.formsElt.children(':not(.deleted)')
appears a lot in the InlinePanel code, and it would be nice to avoid that pattern leaking into MultipleChooserPanel too.)
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.
Thanks, I will try to implement this approach.
Another thing - as per #10496 (comment), we only want to enable this behaviour when we're sure that choosing duplicates should not be allowed. Ideally, it would do this by looking for a unique constraint on the model - but as a first pass, an |
depend on the suggestions, I did :
some fails because I added attribute to hidden input, I will work on this fails with unit tests. if I miss something regarding suggestions or this PR need any modification, please let me know. |
Please use Firstly, there is no guarantee that the chooser linked to MultipleChooserPanel's Secondly - and more importantly - a change such as adding the With |
Great, I will update this PR soon. |
yes make the code or the components independent is better .I updated the PR, so please let me know, if it work |
Fixes #10496
one solution for this issue is disable the duplicate objects or checkboxes, as shown below:
another solution instead of disable the already selected objects, just not add the duplicate objects , when the user add
duplicate object , this object not added to the selection objects. I can update the PR , if this solution is better.