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

[Bug Fix 1288] "Undefined" for nested variables #1637

Merged
merged 5 commits into from Aug 16, 2019

Conversation

@Gabrz
Copy link
Contributor

commented Aug 11, 2019

(Fixes #1288) Extract parent from nunjucks tag when it is a nested property nested. The parent is then used to fetch the keyContext

(Fixes bug #1288) Extract parent from nunjucks tag when it is a nested property nested. The parent is then used to fetch the keyContext
@Gabrz Gabrz changed the title Update nunjucks-tags.js [Bug Update nunjucks-tags.js Aug 11, 2019
@Gabrz Gabrz changed the title [Bug Update nunjucks-tags.js [Bug Fix 1288] "Undefined" for nested variables Aug 11, 2019
Gabrz added 2 commits Aug 12, 2019
Added function that recursively gets the keys. So they will show the correct parent location in the nunjuck tooltip.
Copy link
Collaborator

left a comment

Thanks for tacking a crack at this one @Gabrz! A few comments but nothing too difficult hopefully. Looking forward to getting this merged 🥳

keySource[key] = 'root';
// Function that recursively gets keys
function getKeySource(subObject, inKey, inValue) {
for (let key in subObject) {

This comment has been minimized.

Copy link
@gschier

gschier Aug 12, 2019

Collaborator

Please use const key of Object.keys(subObject) to prevent iterating over the object prototype.

This comment has been minimized.

Copy link
@Gabrz

Gabrz Aug 13, 2019

Author Contributor

Thank you for your feedback! I usually don't program in JavaScript, it helped a lot that you specifically stated what you would like to have changed.

I really like Insomnia, and it would be awesome if my contribution made it into the build!

for (let key in subObject) {
if (typeof subObject[key] === 'string') {
keySource[inKey + key] = inValue;
} else if (typeof subObject[key] === 'object') {

This comment has been minimized.

Copy link
@gschier

gschier Aug 12, 2019

Collaborator

Unfortunately typeof null === 'object' is true as well. Similar to common/render.js, these checks should use a more reliable method.

const asStr = Object.prototype.toString.call(x);

Specifically, they should be changed to the following:

typeof subObject[key] === 'string'
typeof subObject[key] === 'object'

// Should use the following, more reliable, checks instead

Object.prototype.toString.call(subObject[key]) === '[object String]'
Object.prototype.toString.call(subObject[key]) === '[object Object]'
// Function that recursively gets keys
function getKeySource(subObject, inKey, inValue) {
for (let key in subObject) {
if (typeof subObject[key] === 'string') {

This comment has been minimized.

Copy link
@gschier

gschier Aug 12, 2019

Collaborator

What happens if it's a number, array, boolean, or null?

}

// Get Keys from ancestors

This comment has been minimized.

Copy link
@gschier

gschier Aug 12, 2019

Collaborator

Nice work leaving a comment! 😄

for (const key of Object.keys(ancestor.environment || {})) {
keySource[key] = ancestor.name || '';
}
getKeySource(ancestor.environment || {}, '', ancestor.name || '');

This comment has been minimized.

Copy link
@gschier

gschier Aug 12, 2019

Collaborator

Awesome. I like that this logic is grouped together in one place now.

Gabrz added 2 commits Aug 13, 2019
A few little changes after review of the initial change.
Added loop to store Key and Source when item is an Array.
Copy link
Collaborator

left a comment

Nice work @Gabrz! Thanks for contributing 🥳

@gschier gschier merged commit 5e53c82 into getinsomnia:develop Aug 16, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@welcome

This comment has been minimized.

Copy link

commented Aug 16, 2019

Congrats on merging your first pull request! 🎉🎉🎉 You're helping make Insomnia awesome! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.