Skip to content

Add __type property to GeoPoint fields in PostgresStorageAdapter#3695

Merged
flovilmart merged 2 commits into
parse-community:masterfrom
Irus11:master
Apr 7, 2017
Merged

Add __type property to GeoPoint fields in PostgresStorageAdapter#3695
flovilmart merged 2 commits into
parse-community:masterfrom
Irus11:master

Conversation

@dan-dr

@dan-dr dan-dr commented Apr 6, 2017

Copy link
Copy Markdown
Contributor

Resolves #3694

@flovilmart

Copy link
Copy Markdown
Contributor

Thanks for the PR! Could you add a test so we make sure we don't add any regression at a later time?

@codecov

codecov Bot commented Apr 6, 2017

Copy link
Copy Markdown

Codecov Report

Merging #3695 into master will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3695      +/-   ##
==========================================
+ Coverage   90.37%   90.41%   +0.04%     
==========================================
  Files         114      114              
  Lines        7405     7405              
==========================================
+ Hits         6692     6695       +3     
+ Misses        713      710       -3
Impacted Files Coverage Δ
...dapters/Storage/Postgres/PostgresStorageAdapter.js 95.31% <ø> (+0.13%) ⬆️
src/RestWrite.js 94.51% <0%> (+0.2%) ⬆️
src/Controllers/SchemaController.js 97.24% <0%> (+0.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0add7c0...76e31b1. Read the comment docs.

@dan-dr

dan-dr commented Apr 6, 2017

Copy link
Copy Markdown
Contributor Author

No problem, but how should I approach this? i'm finding hard to run on Windows through Cygwin (complains I it can't connect to the mongodb instance) and I can't figure out how to run the test with Postgers. Is there any guide on how to run the tests?

I was thinking of adding something of this sort:

  it('encodes into a JSON with __type field which can be parsed into ParseGeoPoint type', done => {
    var point = new Parse.GeoPoint(44.0, -11.0);
    var obj = new TestObject();
    obj.set('location', point);
    obj.set('name', 'Zhoul')
    obj.save(null, {
      success: () => {
        var query = new Parse.Query(TestObject);
        query.find({
          success: function(results) {
            equal(results.length, 1);
            var pointAgain = results[0].get('location');
            ok(pointAgain);
            ok(pointAgain instanceof Parse.GeoPoint);
            equal(pointAgain.latitude, 44.0);
            equal(pointAgain.longitude, -11.0);
            done();
          }
        });
      }
    })
  });

Do you think this is enough? Could you test this with Postgres and see if it's passing?

@flovilmart

Copy link
Copy Markdown
Contributor

Probably directly against a raw HTTP call, this way we can inspect thé JSON body. What do you think?

@dan-dr

dan-dr commented Apr 6, 2017

Copy link
Copy Markdown
Contributor Author

Sounds good! Just need some guidance on how to run tests with postgres instead of mongo.

@flovilmart

Copy link
Copy Markdown
Contributor

Ah right, did you look into the .travis.yaml file at the root of the repo?

@flovilmart

Copy link
Copy Markdown
Contributor

First you'll need postgres 9.5 with postGIS extension installed.

Then:

  • Create the DB as well as the postgis extension
$ psql -c 'create database parse_server_postgres_adapter_test_database;' -U postgres
$ psql -c 'CREATE EXTENSION postgis;' -U postgres -d parse_server_postgres_adapter_test_database
$ psql -c 'CREATE EXTENSION postgis_topology;' -U postgres -d parse_server_postgres_adapter_test_database
  • run the tests with:
PARSE_SERVER_TEST_DB=postgres npm test

@dan-dr

dan-dr commented Apr 7, 2017

Copy link
Copy Markdown
Contributor Author

Done!
When can I (approximately) expect it to be released? Having to bundle my own version of parse-server makes my deployment process much more cumbersome

@flovilmart

Copy link
Copy Markdown
Contributor

@zhoul-HS I'll try to release a new version before the end of the week. Just need to make sure all relevant PR's are merged before we release :)

@flovilmart flovilmart merged commit 5282868 into parse-community:master Apr 7, 2017
@dan-dr

dan-dr commented Apr 7, 2017

Copy link
Copy Markdown
Contributor Author

Thanks! I'll figure out a workaround in the meantime

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.

2 participants