Skip to content
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

Use selected startDate in layer query #25

Merged
merged 5 commits into from
Apr 30, 2020
Merged

Conversation

ericboucher
Copy link
Collaborator

@ericboucher ericboucher commented Apr 24, 2020

Use startDate info in the layer query.

This surfaced a few issues/needs:

  • need for a better definition of layers. We could probably get rid of the path and use a config for servers, then a server_layer_id and server_style. (Improve config definition for servers and layers #29 )

  • For now, we should only be able to select one layer that has dates at a time. (Limit to one "has_date" layer at a time #28 )

  • handle errors when fetching data. In particular, have the ability to parse time mismatch "Time dimension value '2020-02-10' not valid for this layer". (Error handling for layers #27 )

  • We are currently getting the "Time dimension value '2020-02-10' not valid for this layer" because of a timezone difference which creates a one-day diff. 2020-02-11 is available. We should indicate the timezone in the server definition. (Fixed by adding a moment default timezone)

@ericboucher ericboucher changed the base branch from master to wms-get-date April 24, 2020 23:06
@ericboucher ericboucher changed the title WIP - Use selected startDate in layer query Use selected startDate in layer query Apr 26, 2020
Copy link
Contributor

@mamadOuologuem mamadOuologuem left a comment

Choose a reason for hiding this comment

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

Some small comments

@@ -6,10 +6,16 @@ import DateSelector from '.';
import { store } from '../../../context/store';

test('renders as expected', () => {
const realDateNow = Date.now.bind(global.Date);
Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about mocking the whole store

This shouldn't be a problem anymore because the initial values of dates are number

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a problem because the maxDate keeps changing

@@ -27,6 +27,9 @@ import {
import { months, getMonthStartAndEnd, isAvailableMonth } from './utils';
import { AvailableDates } from '../../../config/types';

// Set default timezone to UTC
Copy link
Contributor

Choose a reason for hiding this comment

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

Do it make sense to set that in src/components/App/index.tsx?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's have you play around with that when fixing #30

src/config/types.ts Outdated Show resolved Hide resolved
@@ -17,10 +17,12 @@ const Map = ReactMapboxGl({
function MapView({ classes }: MapViewProps) {
const layers = useSelector(layersSelector);
const serverAvailableDates = useSelector(availableDatesSelector);
const selectedLayersDates = serverAvailableDates.filter((dates, layerId) =>
layers.has(layerId),
const selectedLayersDates = serverAvailableDates.filter((_, layerId) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

For example in src/config/layers.json:

"ndvi": {
    "title": "Modis NDVI",
    "server_type": "wms",
--> "server_layer": "ModisIndices",
    "server_uri": "http://185.30.8.6:5000/wms?styles=ndvi&layers=ModisIndices",
    "has_date": true,
    "date_interval": "month",
    "opacity": 0.3,
    "legend_text": "Modis NDVI"
},

The layerId is ndvi and server_layer is ModisIndices.

I will keep layers.has(layerId) 🤔?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The names from the server and the layerId are not the same. We can discuss that if thtat's not clear but I will likely change a few things in #29

Copy link
Contributor

@mamadOuologuem mamadOuologuem left a comment

Choose a reason for hiding this comment

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

LGTM!

@ericboucher ericboucher merged commit 75734b1 into wms-get-date Apr 30, 2020
@ericboucher ericboucher added this to Done in PRISM Frontend via automation Apr 30, 2020
@ericboucher ericboucher deleted the manage-dates branch April 30, 2020 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants