Add "off" time by calufa · Pull Request #59 · vlab-research/fly

calufa · 2021-12-07T22:05:33Z

TODO:

add "off" time to surveys
add local test suite to dashboard-server
add local test suite to replybot
add migration script for surveys_metadata table

netlify · 2021-12-07T22:05:42Z

✔️ Deploy Preview for vlab-research canceled.

🔨 Explore the source changes: 5a20d17

🔍 Inspect the deploy log: https://app.netlify.com/sites/vlab-research/deploys/61c3ad556af1e0000751a486

nandanrao · 2021-12-21T04:37:07Z

+CREATE TABLE chatroach.surveys_metadata (
+	surveyid UUID NOT NULL REFERENCES chatroach.surveys(id) ON DELETE CASCADE,
+	off_date TIMESTAMPTZ NOT NULL,
+	UNIQUE(surveyid)


If it's unique and not null, it might be more expresive to just call it a primary key ?

I would call it survey_metadata -- "data" is already plural, and it's not metadata about all the surveys, each row just refers to one survey!

Sounds good.

nandanrao · 2021-12-21T04:41:47Z


 function _response (ctx, qa, {question, validation, response, token, followUp}) {

+  if (ctx.surveyMetadata && ctx.surveyMetadata.off_date < ctx.timestamp) {


Hrm, so even though the table in the database is called "surveys", everywhere here in replybot it's referred to as a "form"... So I would definitely try and keep it consist it within an app.

Wait, no, nevermind, the metadata applies to the whole survey, not just the form. I take it back!

Why is it in reference to ctx.timestamp and not response.timestamp?

nandanrao · 2021-12-21T04:50:42Z

+    const surveyMetadata = await getSurveyMetadata(surveyId)

-    const actions = act({form, user}, state, output)
+    const actions = act({form, user, surveyMetadata, timestamp}, state, output)


I actually don't think act is the right place to put this. The way the event-sourced state machine works, there is exec which does most of the work and creates an intermediate representation called output. That output is then used to transition to the new state via apply and also to generate side effects via act.

But really, when the survey is off, the state should really move to an end state. So the output will need to be different and exec should make the decision.

nandanrao · 2021-12-21T04:51:47Z

Hey mate, overall looks good! But there is one major thing I left a comment on.

nandanrao · 2021-12-22T22:30:39Z



+function exec (state, nxt, surveyMetadata) {
+  if (surveyMetadata && surveyMetadata.off_date < nxt.timestamp) {


Wait, this doesn't make sense to me, we shouldn't respond if the message is a MACHINE_REPORT or a PLATFORM_RESPONSE or an EXTERNAL_EVENT or anything like that.

We can transition to an OFF state, but responding and saying the survey is off should only happen when they write us!

We can, alternatively, make an external_event which is an OFF event, which Dean manages (like timeouts) and then we just react to that event moving everyone into the correct state without having to wait on something else to happen. Maybe that's why it doesn't feel right to find the place now.

nandanrao · 2022-01-06T04:47:54Z

@calufa - how we going on this?

calufa · 2022-01-06T08:16:19Z

Left a comment here, https://curiouslearning.slack.com/archives/D02CDGM9DQB/p1640892843022700?thread_ts=1640792174.000200&cid=D02CDGM9DQB

calufa marked this pull request as draft December 7, 2021 22:05

calufa force-pushed the feature/add-off-time branch 4 times, most recently from e0d1380 to 5584818 Compare December 12, 2021 00:18

add local test suite

701b5ae

calufa force-pushed the feature/add-off-time branch from 5584818 to 701b5ae Compare December 12, 2021 21:05

calufa added 3 commits December 12, 2021 16:39

fix: order responses by timestamp

f426af3

remove dead env vars

b495377

return metadata

9ff415c

calufa force-pushed the feature/add-off-time branch 2 times, most recently from b6dfabe to 49d9c6a Compare December 13, 2021 03:52

add test suite to replybot

da769a7

calufa force-pushed the feature/add-off-time branch 3 times, most recently from 472f975 to 02760c8 Compare December 13, 2021 23:09

add off date logic

fe1f16c

calufa force-pushed the feature/add-off-time branch from 02760c8 to fe1f16c Compare December 19, 2021 04:54

use 0.0.32

a9b537b

calufa commented Dec 20, 2021

View reviewed changes

Comment thread dashboard-server/queries/responses/response.queries.js

calufa commented Dec 20, 2021

View reviewed changes

Comment thread dashboard-server/queries/surveys/survey.test.js

calufa commented Dec 20, 2021

View reviewed changes

Comment thread dashboard-server/queries/surveys/survey.test.js

calufa commented Dec 20, 2021

View reviewed changes

Comment thread devops/sql/migrate-21.sql

calufa requested a review from nandanrao December 20, 2021 19:54

calufa marked this pull request as ready for review December 20, 2021 19:54

calufa force-pushed the feature/add-off-time branch from 24a90b7 to a9b537b Compare December 20, 2021 19:59

nandanrao reviewed Dec 21, 2021

View reviewed changes

Comment thread dashboard-server/config/index.js

nandanrao reviewed Dec 21, 2021

View reviewed changes

Comment thread formcentral/server.go Outdated

nandanrao reviewed Dec 21, 2021

View reviewed changes

calufa force-pushed the feature/add-off-time branch 6 times, most recently from 0678795 to 891096d Compare December 22, 2021 22:18

calufa commented Dec 22, 2021

View reviewed changes

Comment thread replybot/lib/typewheels/transition.js

calufa force-pushed the feature/add-off-time branch 3 times, most recently from dc2b77f to 60357fd Compare December 22, 2021 22:29

nandanrao requested changes Dec 22, 2021

View reviewed changes

calufa force-pushed the feature/add-off-time branch 3 times, most recently from efe6a0c to ac51735 Compare December 22, 2021 22:42

compute off in exec()

5a20d17

calufa force-pushed the feature/add-off-time branch from ac51735 to 5a20d17 Compare December 22, 2021 22:57

calufa mentioned this pull request Jan 7, 2022

Add test suite #66

Open

9 tasks


		function _response (ctx, qa, {question, validation, response, token, followUp}) {

		if (ctx.surveyMetadata && ctx.surveyMetadata.off_date < ctx.timestamp) {



		function exec (state, nxt, surveyMetadata) {
		if (surveyMetadata && surveyMetadata.off_date < nxt.timestamp) {

Uh oh!

Conversation

calufa commented Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nandanrao commented Dec 21, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nandanrao commented Jan 6, 2022

Uh oh!

calufa commented Jan 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

calufa commented Dec 7, 2021 •

edited

Loading

netlify Bot commented Dec 7, 2021 •

edited

Loading