You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by vladimir-kotikov <gi...@git.apache.org> on 2015/11/25 11:40:20 UTC
[GitHub] cordova-plugin-contacts pull request: CB-5308 Return rawId instead...
GitHub user vladimir-kotikov opened a pull request:
https://github.com/apache/cordova-plugin-contacts/pull/87
CB-5308 Return rawId instead of id when modifying existing contact
This fixes issue with updating existing contact, described in [CB-5308](https://issues.apache.org/jira/browse/CB-5308)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-plugin-contacts CB-5308
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-plugin-contacts/pull/87.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #87
----
commit 0c9a0cb616f3de9d3b7a6ea77a5cf543f0e3f13b
Author: Vladimir Kotikov <v-...@microsoft.com>
Date: 2015-11-23T14:18:29Z
CB-5308 Return rawId instead of id when modifying existing contact
commit 03dc3b8b63873f2959b8bac5cf643b4d519d8c6f
Author: Vladimir Kotikov <v-...@microsoft.com>
Date: 2015-11-23T15:52:01Z
CB-5308 Makes contacts save specs passing
* Updates save specs to isolate them from each other
* Removes interference between specs and after-spec cleanup routines
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-contacts pull request: CB-5308 Return rawId instead...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-contacts/pull/87#discussion_r45991178
--- Diff: src/android/ContactAccessorSdk5.java ---
@@ -1027,7 +1027,7 @@ else if (accounts.length > 1) {
private String modifyContact(String id, JSONObject contact, String accountType, String accountName) {
// Get the RAW_CONTACT_ID which is needed to insert new values in an already existing contact.
// But not needed to update existing values.
- int rawId = (Integer.valueOf(getJsonString(contact, "rawId"))).intValue();
+ String rawId = getJsonString(contact, "rawId");
--- End diff --
Should the type be String or long as stated in [the docs](http://developer.android.com/intl/ru/reference/android/provider/ContactsContract.Data.html)? Or does not this matter?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-contacts pull request: CB-5308 Return rawId instead...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-contacts/pull/87#discussion_r46023545
--- Diff: src/android/ContactAccessorSdk5.java ---
@@ -1027,7 +1027,7 @@ else if (accounts.length > 1) {
private String modifyContact(String id, JSONObject contact, String accountType, String accountName) {
// Get the RAW_CONTACT_ID which is needed to insert new values in an already existing contact.
// But not needed to update existing values.
- int rawId = (Integer.valueOf(getJsonString(contact, "rawId"))).intValue();
+ String rawId = getJsonString(contact, "rawId");
--- End diff --
From my experiments this works both with `int` and `String`. My guess is that ContentProvider casts types implicitly, when needed.
Also, according to the [same documentation](http://developer.android.com/intl/ru/reference/android/provider/ContactsContract.Data.html) `id` is also of type `long`, but is has been used here as `String` always. So IMO it doesn't matter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-contacts pull request: CB-5308 Return rawId instead...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-contacts/pull/87#discussion_r46023669
--- Diff: tests/tests.js ---
@@ -37,23 +37,34 @@ exports.defineAutoTests = function() {
var MEDIUM_TIMEOUT = 30000;
- var removeContact = function() {
- if (gContactObj) {
- gContactObj.remove(function() {}, function() {
- console.log("[CONTACTS ERROR]: removeContact cleanup method failed to clean up test artifacts.");
- });
- gContactObj = null;
- }
- };
+ var removeContact = function(done) {
+ if (!done) {
+ done = function noop () { };
--- End diff --
I just realized that `removeContact` is now used only in async way (with `done`) so this is not needed at all. Removing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-contacts pull request: CB-5308 Return rawId instead...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-plugin-contacts/pull/87
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-contacts pull request: CB-5308 Return rawId instead...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-contacts/pull/87#discussion_r45990446
--- Diff: tests/tests.js ---
@@ -358,44 +380,68 @@ exports.defineAutoTests = function() {
pending();
}
- expect(gContactObj).toBeDefined();
-
- var bDay = new Date(1975, 5, 4);
+ var aDay = new Date(1976, 6, 4);
+ var bDay;
var noteText = "an UPDATED note";
- var win = function(obj) {
- expect(obj).toBeDefined();
- expect(obj.id).toBe(gContactObj.id);
- expect(obj.note).toBe(noteText);
- expect(obj.birthday.toDateString()).toBe(bDay.toDateString());
- expect(obj.emails.length).toBe(1);
- expect(obj.emails[0].value).toBe('here@there.com');
- removeContact(); // Clean up contact object
- done();
+ var obj = {
+ "gender": "male",
+ "note": "my note",
+ "name": {
+ "familyName": "Delete",
+ "givenName": "Test"
},
- fail = function() {
- removeContact();
- fail(done);
- };
+ "emails": [{
+ "value": "here@there.com"
+ }, {
+ "value": "there@here.com"
+ }],
+ "birthday": aDay
+ };
+
+ var saveFail = fail.bind(null, done);
+
+ var saveSuccess = function(obj) {
+ // must store returned object in order to have id for update test below
+ gContactObj = obj;
+ gContactObj.emails[1].value = "";
+ bDay = new Date(1975, 5, 4);
+ gContactObj.birthday = bDay;
+ gContactObj.note = noteText;
+ gContactObj.save(updateSuccess, saveFail);
+ };
+
+ var updateSuccess = function(obj) {
+ expect(obj).toBeDefined();
+ expect(obj.id).toBe(gContactObj.id);
+ expect(obj.note).toBe(noteText);
+ expect(obj.birthday.toDateString()).toBe(bDay.toDateString());
+ expect(obj.emails.length).toBe(1);
+ expect(obj.emails[0].value).toBe('here@there.com');
+ done();
+ };
+
+ navigator.contacts
+ .create(obj)
+ .save(saveSuccess, saveFail);
- // remove an email
- gContactObj.emails[1].value = "";
- // change birthday
- gContactObj.birthday = bDay;
- // update note
- gContactObj.note = noteText;
- gContactObj.save(win, fail);
}, MEDIUM_TIMEOUT);
});
+
describe('Contact.remove method', function(done) {
- afterEach(removeContact);
+ afterEach(function (done) {
+ removeContact(done);
+ });
it("contacts.spec.22 calling remove on a contact has an id of null should return ContactError.UNKNOWN_ERROR", function(done) {
- var win = function() {};
+ var win = function() {
+ expect(false).toBe(true);
--- End diff --
IMO this might be rewritten to use the global `fail` function as in spec.23 and others, i.e. use `fail.bind` as it does the same thing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-contacts pull request: CB-5308 Return rawId instead...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-contacts/pull/87#discussion_r45989334
--- Diff: tests/tests.js ---
@@ -37,23 +37,34 @@ exports.defineAutoTests = function() {
var MEDIUM_TIMEOUT = 30000;
- var removeContact = function() {
- if (gContactObj) {
- gContactObj.remove(function() {}, function() {
- console.log("[CONTACTS ERROR]: removeContact cleanup method failed to clean up test artifacts.");
- });
- gContactObj = null;
- }
- };
+ var removeContact = function(done) {
+ if (!done) {
+ done = function noop () { };
--- End diff --
A comment on why this is added would be useful.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org