Skip to content

Bug/sdk 2455/fix sdk for turkish servers #720

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions integrations/clevertap/HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@

1.0.4 / 2022-12-14
* Updates clevertap sdk url

1.0.2 / 2017-08-30
==================

Expand Down
13 changes: 4 additions & 9 deletions integrations/clevertap/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

var integration = require('@segment/analytics.js-integration');
var useHttps = require('use-https');
var each = require('@ndhoule/each');
var is = require('is');

Expand All @@ -17,11 +16,8 @@ var CleverTap = (module.exports = integration('CleverTap')
.global('clevertap')
.option('clevertap_account_id', '')
.option('region', '')
.tag('http', '<script src="http://static.clevertap.com/js/a.js">')
.tag(
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a logic further down the code (in initialize method) that does this:

  var protocol = useHttps() ? 'https' : 'http';
  this.load(protocol, this.ready);

the this.load method uses protocol to decide which script to load, but with this change we removed the https script, and therefore, the this.load will fail for https cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi @pooyaj , If i understood this right, then the this.load will fail for http, since in the tag I've changed it to https now. The intent is to load the script over https only, since most of the people use https.

Also I think use-https library no longer serves the purpose, so would be removing this due to above stated reasons.

Thanks!

'https',
'<script src="https://d2r1yp2w7bby2u.cloudfront.net/js/a.js">'
));
.tag('https', '<script src="https://static.clevertap.com/js/clevertap.min.js">')
);

/**
* Initialize.
Expand All @@ -47,8 +43,7 @@ CleverTap.prototype.initialize = function() {
// and since dealing with mongo is much more painful, we will strip here
window.clevertap.region = region.replace('.', '');
}
var protocol = useHttps() ? 'https' : 'http';
this.load(protocol, this.ready);
this.load('https', this.ready);
};

CleverTap.prototype.loaded = function() {
Expand Down Expand Up @@ -83,7 +78,7 @@ CleverTap.prototype.identify = function(identify) {
each(function(value, key) {
if (!is.object(value)) supportedTraits[key] = value;
}, traits);
window.clevertap.profile.push({
window.clevertap.onUserLogin.push({
Site: supportedTraits
});
};
Expand Down
2 changes: 1 addition & 1 deletion integrations/clevertap/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@segment/analytics.js-integration-clevertap",
"description": "The CleverTap analytics.js integration.",
"version": "1.0.3",
"version": "1.0.4",
"keywords": [
"analytics.js",
"analytics.js-integration",
Expand Down