Skip to content
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

Add update list #376

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AgustinSuarez
Copy link
Contributor

This PR allows sending references to other record type when using the initialize call. For example SO to Item Fulfillment, you can specify which ship group you want NS to return to you. Thats done via an aux_reference.

My mistake I also committed all those new changes in the same branch as the Update List PR that was done a couple months ago. That code remains unchanged.

hash[:attributes!] ||= {}
hash[:attributes!][kname] ||= {}
hash[:attributes!][kname]['replaceAll'] = v.replace_all
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AgustinSuarez why did this functionality need to be moved out of the sublist module? I believe replaceAll is only applicable to sublists, so I don't think it should be required anywhere else. What am I missing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in order to upsert sales orders and other type of transaction correctly, you need to be able to define it as follows <listRel:addressbookList replaceAll="false"/> The problem with the previous code is that it added it as a field and didn't take it into consideration.

This helps when upserting records like addresses and line items

@iloveitaly
Copy link
Member

@AgustinSuarez This looks great! I just left one comment in the code. Two other items that came to mind:

  • Can you rebase this to master?
  • Would be awesome to get a readme entry with some usage examples for the community.

Thanks again!

@AgustinSuarez
Copy link
Contributor Author

Hey @iloveitaly It's already rebased on master. I did on my machine and there are no changes done. I have also added the new documentation

Thank you

@AgustinSuarez
Copy link
Contributor Author

@iloveitaly My mistake. Had an issue with git. I have rebased on master. Thank you!

@iloveitaly iloveitaly force-pushed the master branch 2 times, most recently from 7185ffc to df796ee Compare May 28, 2019 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants