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

Redis node get not editing items #121

Closed
wants to merge 1 commit into from
Closed

Conversation

agix
Copy link
Contributor

@agix agix commented Nov 13, 2019

When using redis node with Get or Keys operation, propertyName is not populated.

An item variable is created with empty json attribute which is then populated on propertyName. But this variabe is never used later.

I don't understand why this variable exist so I removed it and directly edit items[itemIndex].

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Florian GAULTIER seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

janober added a commit that referenced this pull request Nov 13, 2019
@janober
Copy link
Member

janober commented Nov 13, 2019

Thanks a lot for reporting the issue!

However, had to fix that a little bit differently. Return now the items like they were supposed to. The reason why I could not accept your PR is that it changes the incoming data which is not allowed. It is described here:
https://docs.n8n.io/#/create-node?id=do-not-change-incoming-data

If it does not describe good enough please simply get back to me.

Thanks for your help!

@janober janober closed this Nov 13, 2019
@agix
Copy link
Contributor Author

agix commented Nov 14, 2019

Oh ok, will keep this in mind. I did agix@62d50cb but I just saw you already fixed it.

@janober
Copy link
Member

janober commented Nov 14, 2019

Sorry for the unnecessary work. Was probably not making it very clear that I had fixed it.
I was in the process of releasing a new version anyway so I thought I directly fix it that it works for you and everybody else.

@agix
Copy link
Contributor Author

agix commented Nov 14, 2019

No problem :) thank you very much for your quick fix !

Azmah-Bad pushed a commit to Azmah-Bad/n8n that referenced this pull request Jan 9, 2023
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.

3 participants