koopjs / FeatureServer

An open source Geoservices Implementation (deprecated)
https://geoservices.github.io
Other
101 stars 32 forks source link

Update templates.js #162

Closed ButchGis closed 4 years ago

ButchGis commented 4 years ago

added check for metadata.renderer to allow a koop provider to provide symbology for feature server route.

This should satisfy issue #80 and maybe #65 but, #65 might be asking for more than this.

hparkertt commented 4 years ago

@rgwozdz and/or @ButchGis Any progress on this? I think this is something that would be a huge win for the koop project.

rgwozdz commented 4 years ago

I think we are just waiting on unit tests. @ButchGis - if you want to hand this off, let me know. I can branch and finish the tests if that helps.

hparkertt commented 4 years ago

@rgwozdz Is e8587f1 not what you're looking for? I'm not sure if you were expecting more testing, but from reading this thread I would have assumed that was sufficient.

rgwozdz commented 4 years ago

It's not enough, and I have been in touch with @ButchGis directly to describe the need. But to summarize, https://github.com/koopjs/FeatureServer/commit/e8587f10abca3b1455e746d472f2d9af7f45273f simply adds a renderer to the test fixture. The existing test only tests whether the renderer in use fits the expected schema; i.e., it doesn’t test any values of the renderer itself. So with the existing test we don't know if the values from the fixture have been applied as we expect.

We need to add some assertions that indicate the custom renderer has been applied (as opposed to the default renderer).

So we need some assertions like:

layers.layers[0].drawiningInfo.renderer.symbol.color[0].should.equal(115)
layers.layers[0].drawiningInfo.renderer.symbol.color[1].should.equal(76)
layers.layers[0].drawiningInfo.renderer.symbol.color[2].should.equal(0)
layers.layers[0].drawiningInfo.renderer.symbol.color[3].should.equal(255)

right about here. Are you interested in helping us finish this off?

hparkertt commented 4 years ago

I can try and tackle that tomorrow. I forked from his repo and ran the test, noticing there were a few errors that don't appear to be related to this issue specifically. I'm assuming I can push a commit with the new assertions and ignore those errors?

rgwozdz commented 4 years ago

Warning can be ignored, but any failed tests will prevent an eventual merge. What are you seeing?

ButchGis commented 4 years ago

I can get to those asserts this evening. I did not realize any urgency before. Thought there might be other things still in the works before next updates pushed to core. Should not take to long to do a few asserts.

@hparkett, I had some other testing issues and warnings as well. Though I am pretty sure I had some environment issues and such. I wasn't set for testing at all until Rich prompted me for this test a few weeks back.


From: Rich Gwozdz notifications@github.com Sent: Thursday, September 26, 2019 3:11 PM To: koopjs/FeatureServer FeatureServer@noreply.github.com Cc: Anthony Fragale AFragale@esri.com; Mention mention@noreply.github.com Subject: Re: [koopjs/FeatureServer] Update templates.js (#162)

Warning can be ignored, but any failed tests will prevent an eventual merge. What are you seeing?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_koopjs_FeatureServer_pull_162-3Femail-5Fsource-3Dnotifications-26email-5Ftoken-3DAH2TAVYKSG2G3D4K3DYOXKLQLUCPDA5CNFSM4ITI5L22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7WU5RY-23issuecomment-2D535645895&d=DwMCaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=CjzKn4LMK8XMlUn2RqpGT4XBjFCdyudPrnhO5cbl08k&m=xXujmG8ZHSFkvaiU9PsxLuEKGxUgBwqzTpoukuv1LcM&s=N4Fb5SlIPWo22A1iZijC-Xs8ZUphSQdrHFqz6OkWei4&e=, or mute the threadhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AH2TAV6HMC7J6GU77PUDQS3QLUCPDANCNFSM4ITI5L2Q&d=DwMCaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=CjzKn4LMK8XMlUn2RqpGT4XBjFCdyudPrnhO5cbl08k&m=xXujmG8ZHSFkvaiU9PsxLuEKGxUgBwqzTpoukuv1LcM&s=D2B_c9nCSchv5QWZOSXapIYphmq3L3lVedk9J4AnIuc&e=.

hparkertt commented 4 years ago

Awesome, thanks guys! I'm looking forward to testing this out once it hits a release!

rgwozdz commented 4 years ago

released in 2.21.0