Skip to content

Conversation

@doga-foton
Copy link

@doga-foton doga-foton commented Oct 29, 2021

It's a work in progress.

Related to:
OV-1262
OV-1261

API:

  • API endpoint has been added
  • Query to get trades with relational data to list on UI

UI:

  • Menu item has been added
  • View has been added

image

I would like to get feedback about;

  • If the implementation of the UI applied to guidelines.
  • I needed to add two controllers on /trade/exchange and /trade/exchange-irec packages.
  • Existing trade.service has been used, and made getAll method as public, should I create a service only for this case?
  • Folder / file structure

@CLAassistant
Copy link

CLAassistant commented Oct 29, 2021

CLA assistant check
All committers have signed the CLA.

@doga-foton doga-foton marked this pull request as draft October 29, 2021 08:42
Copy link
Contributor

@alexworker23 alexworker23 left a comment

Choose a reason for hiding this comment

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

In general: the implementation of the UI applies to guidelines ✅

const formatTrades: TFormatTrades = ({ trades, allDevices }) => {
const formattedTrades: FormattedTrade[] = [];

trades?.forEach((trade) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

const formattedTrades: FormattedTrade[] = trades.map will better fit here

date: t('admin.trades.date'),
buyer: t('admin.trades.buyer'),
seller: t('admin.trades.seller'),
energy: 'Energy',
Copy link
Contributor

Choose a reason for hiding this comment

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

please don't forget to localize this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants