Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

"Make the context correct" part of nested_form.js (jQuery) #179

Closed
LucasLMartini opened this Issue Jul 15, 2012 · 4 comments

Comments

Projects
None yet
2 participants

What purpose serves the following part of nested_form.js?

      // Make the context correct by replacing new_<parents> with the generated ID
      // of each of the parent objects
      var context = ($(link).closest('.fields').find('input:first').attr('name') || '').replace(new RegExp('\[[a-z]+\]$'), '');

      // context will be something like this for a brand new form:
      // project[tasks_attributes][new_1255929127459][assignments_attributes][new_1255929128105]
      // or for an edit form:
      // project[tasks_attributes][0][assignments_attributes][1]
      if (context) {
        var parentNames = context.match(/[a-z_]+_attributes/g) || [];
        var parentIds   = context.match(/(new_)?[0-9]+/g) || [];

        for(var i = 0; i < parentNames.length; i++) {
          if(parentIds[i]) {
            content = content.replace(
              new RegExp('(_' + parentNames[i] + ')_.+?_', 'g'),
              '$1_' + parentIds[i] + '_');

            content = content.replace(
              new RegExp('(\\[' + parentNames[i] + '\\])\\[.+?\\]', 'g'),
              '$1[' + parentIds[i] + ']');
          }
        }
      }

I was having an issue where I wanted to allow the user to insert child objects anywhere in the form (the order of which was important for the parent model and also for the user) and this code would clone the nearest child's attributes (children that were already present). Definitely not the intended behavior in my case, as I was adding new children.

I simply commented the aforementioned code and it started working, allowing me to insert new children in any position in the form.

Question is: What is that code for? I couldn't really figure it out from just reading it and everything continued working properly after I commented it...

Ok, I understand what the code is for now, did more research.
It's used to set the correct IDs for multiple nested forms, correct?

To avoid issues with context when that is not the case, I did the following:

     if (context) {
        var parentNames = context.match(/[a-z_]+_attributes/g) || [];
        var parentIds   = context.match(/(new_)?[0-9]+/g) || [];

        for(var i = 0; i < parentNames.length; i++) {
          if(parentIds[i] && (parentNames != (assoc+"_attributes"))) {
            content = content.replace(
              new RegExp('(_' + parentNames[i] + ')_.+?_', 'g'),
              '$1_' + parentIds[i] + '_');

            content = content.replace(
              new RegExp('(\\[' + parentNames[i] + '\\])\\[.+?\\]', 'g'),
              '$1[' + parentIds[i] + ']');
          }
        }
      }

Note the larger if inside the for...

That way we make sure we're ignoring IDs when parentNames are equal to the association that is currently being inserted. After all, it doesn't make much sense (can anyone come up with a real use-case?) that a model should "has_many" of itself, right?

This will make nested_form work nicely when you want to be able to insert children in the middle as opposed to appending them at the end always...

Should I fork this?

Collaborator

lest commented Aug 23, 2012

I guess the latest changes may fix your issue. Could you please test against the latest version and push a test application if there is still an issue? Thanks!

Apparently it does not, same issue. I see the latest .js for jQuery hasn't changed where I mentioned, the identification of the parent would probably have to change.

Mine is a very specific corner-case I suppose though. I will try to trim down my app to a workable example if you guys wanna shoot at it; later today or tomorrow I'll comment back here.

Cheers!

Collaborator

lest commented Sep 16, 2012

Closing for now. I'll reopen if a test application reproducing this issue is provided.

@lest lest closed this Sep 16, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment