这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@wawhal
Copy link
Contributor

@wawhal wawhal commented May 11, 2020

Description

This PR does the following:

  1. Add UI for Scheduled Trigers
  2. Rewrite Event Triggers in TypeScript
  3. Refactors some common files to TypeScript
  4. Moves ET and ST under a single service called Events

Certain TypeScript patterns that have been followed:

  1. any has been used as a type for the following:

    • dispatch: Can't think of a better way until we have a union type that encompasses all redux actions and we have a way to handle thunk. Suggestions are welcome
    • Row of a table (eg: while rendering React table)
    • Variable payloads (eg: request/response of ET/ST invocations)
    • Generally the things that are variable values coming from the server
  2. Every component has its prop type next to it. All components will have the following structure.

type Props = {
  dispatch: any,
  currentTrigger: ScheduledTrigger
};

const ModifyScheduledTrigger: React.FC<Props> = (props) => {
  const { dispatch, currentTrigger } = props;

  // some transformations

  return <JSX />
}

Notes for the reviewer:

  1. The UI for Event Triggers (JSX) has been taken as is from the existing codebase and not much changes are made; there's definitely a scope for improvement from UI POV, but that's for a separate PR.

  2. src/components/Common/utils have been converted to TS. A detailed review on these would be nice because these utils are more or less the core of the app.

  3. API has changed, so the full functionality feedback might be redundant for now.

  4. Use this docker image ( hasura/graphql-engine:pull3553-0134b257)

Changelog

  • CHANGELOG.md is updated with user-facing content relevant to this PR.

Affected components

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System
  • Tests
  • Other (list it)

@wawhal wawhal requested a review from beerose May 11, 2020 10:36
@netlify
Copy link

netlify bot commented May 11, 2020

Deploy preview for hasura-docs ready!

Built with commit 99cc02b

https://deploy-preview-4732--hasura-docs.netlify.app

@hasura-bot
Copy link
Contributor

Review app for commit cf3d1ba deployed to Heroku: https://hge-ci-pull-4732.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4732-cf3d1ba2

@wawhal wawhal force-pushed the console-scheduled-triggers branch from e30e416 to 28174b7 Compare May 12, 2020 09:11
@hasura-bot
Copy link
Contributor

Review app for commit 28174b7 deployed to Heroku: https://hge-ci-pull-4732.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4732-28174b78

@wawhal wawhal requested a review from a team as a code owner May 29, 2020 14:42
@hasura-bot
Copy link
Contributor

Review app for commit 45f6e58 deployed to Heroku: https://hge-ci-pull-4732.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4732-45f6e58f

@wawhal wawhal force-pushed the console-scheduled-triggers branch from 45f6e58 to c7ec515 Compare May 29, 2020 15:30
@hasura-bot
Copy link
Contributor

Review app for commit c7ec515 deployed to Heroku: https://hge-ci-pull-4732.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4732-c7ec515c

@rikinsk
Copy link
Member

rikinsk commented Jun 1, 2020

/heroku deploy

2 similar comments
@rikinsk
Copy link
Member

rikinsk commented Jun 1, 2020

/heroku deploy

@rikinsk
Copy link
Member

rikinsk commented Jun 4, 2020

/heroku deploy

@hasura-bot
Copy link
Contributor

Review app for commit 82a22ec deployed to Heroku: https://hge-ci-pull-4732.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4732-82a22ec0

@hasura-bot
Copy link
Contributor

Review app for commit 277f87d deployed to Heroku: https://hge-ci-pull-4732.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4732-277f87d3

@hasura-bot
Copy link
Contributor

Review app for commit 0122aa0 deployed to Heroku: https://hge-ci-pull-4732.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4732-0122aa00

@rikinsk rikinsk removed the request for review from a team June 5, 2020 07:26
@rikinsk rikinsk removed the s/do-not-merge Do not merge this pull request to master label Jun 5, 2020
@hasura-bot
Copy link
Contributor

Review app for commit 0b953b3 deployed to Heroku: https://hge-ci-pull-4732.herokuapp.com
Docker image for server: hasura/graphql-engine:pull4732-0b953b3b

@rikinsk rikinsk merged commit aaab6d3 into hasura:master Jun 5, 2020
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-4732.herokuapp.com is deleted

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants