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

Proposal: new /sources folder #164

Closed
Clebal opened this issue Jan 20, 2021 · 7 comments
Closed

Proposal: new /sources folder #164

Clebal opened this issue Jan 20, 2021 · 7 comments

Comments

@Clebal
Copy link
Contributor

Clebal commented Jan 20, 2021

To allow user to define sources easily, the Front-PS team proposes to create a new /sources folders where the definition of the sources are setted. This new folder will be encountered inside a new /data folder that will also contains the /models. The data folder and its structure is aimed to split the front / back parts of the application.

└── data
    ├── models
    └── sources

What's inside sources folder?

Inside the sources folder, you can find multiple JS files named with the following structure: somethingSource.js. Each file contains one source definition beside the definition of its related widgets. It's more easy to dive inside with an example:

This is the KpiSource.js:

// Firstly, we define the ID of the source
export const KPI_SOURCE_ID = 'kpiSource'

// Use constants for query columns aliases to
// avoid problem with future changes.
export const KPI_SOURCE_COLUMNS = {
  NAME: 'name',
  REVENUE: 'revenue'
}

// Define the source structure that will be passed to `addSource` reducer
// Remember: it's important to use aliases with the constants defined above.
export const kpiSource = {
  id: KPI_SOURCE_ID,
  data: `
  SELECT
    states.name as ${KPI_SOURCE_COLUMNS.NAME},
    SUM(stores.revenue) as ${KPI_SOURCE_COLUMNS.REVENUE},
    states.the_geom_webmercator
  FROM ne_50m_admin_1_states as states
  JOIN retail_stores as stores ON ST_Intersects(states.the_geom_webmercator, stores.the_geom_webmercator)
  GROUP BY states.name, states.the_geom_webmercator
  `,
}

// After the definition of the source, define the widgets properties
// needed to make them work. In this case, we have two widgets:
export const KPI_SOURCE_WIDGETS = {
  TOTAL_REVENUE: {
    column: KPI_SOURCE_COLUMNS.REVENUE,
    operation: AggregationTypes.SUM
  },
  REVENUE_BY_STATE: {
    column: KPI_SOURCE_COLUMNS.NAME,
    operationColumn: KPI_SOURCE_COLUMNS.REVENUE,
    operation: AggregationTypes.SUM
  }
}
@VictorVelarde
Copy link
Contributor

Hi @Clebal!

I like the idea of a /data/sources. Just to have a clear idea of the other folder what contents do you propose for /data/models? can you put also an example?

Regarding to source file contents:

  • 👍🏻 like having the ID, the SOURCE_COLUMNS and the SOURCE itself
  • ❓ I have doubts about including the WIDGETS inside: if the idea is to split backend from front, shouldn't those widget properties be outside, as they are more related to the visual/frontend part? what do you think?

@juandjara
Copy link

juandjara commented Jan 20, 2021

I'm following this same structure in the site planning product project and in there, data/sources contains functions that return strings (sql strings for sources) while data/models contains functions that call the SQL API directly with executeQuery and return or process those results

@aaranadev
Copy link
Collaborator

@VictorVelarde , KPI_SOURCE_WIDGETS is the interface for the frontend, we don't want to know the column name and the operation about that column. We prefer a key and backend includes the operation and name column like they want

@VictorVelarde
Copy link
Contributor

Ok, if it works for you and it fits the workflow, I'm ok with the SOURCE_WIDGETS side.

@albertoarana do you aggree on keeping the division between data/sources (declarative, for layers & widgets), from data/models (for custom queries against any api?)

@aaranadev
Copy link
Collaborator

I agree, maybe that X_WIDGETS won't be the best suffix. I would try with other name.

Any suggestion?

@aaranadev
Copy link
Collaborator

We will go to remove _WIDGETS from source finally

@VictorVelarde
Copy link
Contributor

Already done

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

No branches or pull requests

4 participants