Skip to content

Fix email and self_entity changing ids server-side. And initial conv list.#104

Merged
averissimo merged 3 commits intoyakyak:masterfrom
HomerSp:work
Oct 26, 2018
Merged

Fix email and self_entity changing ids server-side. And initial conv list.#104
averissimo merged 3 commits intoyakyak:masterfrom
HomerSp:work

Conversation

@HomerSp
Copy link
Copy Markdown
Contributor

@HomerSp HomerSp commented Oct 24, 2018

As the title says, this fixes the ids of email and self_entity, as well as fixing conversation states no longer being sent in the init message by running sync after initializing.
Wasn't sure what to do about conv_states being initialized to ds:20 still, so I left it as it doesn't seem to do any harm.

This fixes #103

Fix conversation states no longer being sent in the init message by running sync after initializing.
test/body.html Outdated
]
]
}});</script><script>AF_initDataCallback({key: 'ds:32', isError: false , hash: '29', data:function(){return [["cib:sc",3600,3600,3,600,10]
}});</script>><script>AF_initDataCallback({key: 'ds:32', isError: false , hash: '29', data:function(){return [["cib:sc",3600,3600,3,600,10]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks like an extra <

]
}});</script><script>AF_initDataCallback({key: 'ds:31', isError: false , hash: '28', data:function(){return [["cic:sc",["AF","DZ","AO","BH","BD","BJ","BF","KH","CM","CG","CD","CI","GH","GU","GN","ID","IQ","IL","JO","KZ","KE","KW","KG","LA","LR","MG","MW","MY","MV","MA","MZ","NE","NG","OM","PK","PS","PH","SA","SN","SC","SL","SO","ZA","LK","TZ","TH","TG","TN","TR","UG","UA","UZ","VN","ZM"]
]
}});</script><script>AF_initDataCallback({key: 'ds:31', isError: false , hash: '37', data:function(){return [["cic:vd","104889609452098951178","joao.marques.antunes@gmail.com",0,""]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What is the significance of 104889609452098951178 and joao.marques.antunes@gmail.com here?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I edited the patch and left the original text on the line and am able to connect.

Copy link
Copy Markdown

@plessbd plessbd Oct 25, 2018

Choose a reason for hiding this comment

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

@dmidthun that is because this wont effect the running of the program, just the tests.

@smammy I assumed it was so that test would pass unlike #102 which fails https://travis-ci.org/yakyak/hangupsjs/builds/445768280

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

OK, got it. Thanks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since the id of email was changed it had to be changed in body.html as well for it to pass, so I just copied over the data from the old id to the new one.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The email is from a fake account that I used to generate the tests manually.

The tests need to be greatly improved as they do close to nothing.

@cgatesman
Copy link
Copy Markdown

Please do not comment on pull requests if you don't know what you are doing or how to apply and test it properly.

@smammy
Copy link
Copy Markdown

smammy commented Oct 25, 2018

@psouza4 @edraven this is not the place to ask for tech support. Please refer to yakyak/yakyak#981 (comment)

@nicholashudson2
Copy link
Copy Markdown

I was able to apply the changes in the three commits associated with this pull request, compile, and run without any issues. Connection was established quickly, message history populated as expected, and no error messages were encountered. Have been testing off and on throughout the day today, and so far, it's been smooth sailing. @HomerSp Good work! Problem appears to be resolved, tests are passing consistently, and I've yet to encounter any errors/exceptions.

@Mange
Copy link
Copy Markdown

Mange commented Oct 26, 2018

I manually modified the compiled code with what I think the resulting JS would look like, and I can also confirm that this works for me. Thank you!

(It's been some time since I wrote CoffeeScript, but I could also use the code around it to remind me of how the JS should look like.)

@blondehouse
Copy link
Copy Markdown

Hi all,
I'm brand new to coding and understanding patches. Can someone walk me through on how I can fix this same issue i've been having? I'm running 1.5.2 on Mojave.
Thanks!

@averissimo averissimo merged commit 2bc6c76 into yakyak:master Oct 26, 2018
@smammy
Copy link
Copy Markdown

smammy commented Oct 26, 2018

@blondehouse this fix is in a subcomponent and not a straightforward patch so you're probably better off waiting for the YakYak release (which looks like it's in progress).

@averissimo
Copy link
Copy Markdown
Member

@smammy exactly!

@blondehouse we're doing the necessary steps to release a new yakyak version in the next hour or so

@blondehouse
Copy link
Copy Markdown

@smammy @averissimo Thank you guys!!

@averissimo
Copy link
Copy Markdown
Member

@HomerSp @smammy @akholodenko thanks for the patch and peer review of code. We appreciate so much.

We'd welcome more patches and new blood in hangupsjs ;)

@HomerSp
Copy link
Copy Markdown
Contributor Author

HomerSp commented Oct 27, 2018

You're very welcome - glad I could help! If I do any more work on it I will certainly send in more pull requests.

@HomerSp HomerSp deleted the work branch November 1, 2018 09:30
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.

broken again?

10 participants