git ssb - Merge pull request #4 from ssbc/position_review

add poll property to positions. perhaps it should be `root` instead?

Piet Geursen authored on 3/5/2018, 9:55:18 PM
GitHub committed on 3/5/2018, 9:55:18 PM
Parent: 07a5f24f2034628892c7c9ed0782b63b30fc7911
Parent: 89f12570ac64ff180e7f40f85d7bf29a33a17e1d

README.md	changed
position/schema/position.js	changed
position/sync/chooseOne.js	changed
position/sync/position.js	changed
test/position/sync/chooseOne.test.js	changed
test/position/sync/isPosition.test.js	changed
types.js	changed

position/schema/position.jsView
		@@ -4,9 +4,9 @@
4	4
5	5	const schema = {
6	6	$schema: 'http://json-schema.org/schema#',
7	7	type: 'object',
8		- required: ['type', 'positionDetails'],
	8	+ required: ['type', 'poll', 'positionDetails'],
9	9	properties: {
10	10	version: {
11	11	type: 'string',
12	12	pattern: '^0.1.0$'
		@@ -14,8 +14,11 @@
14	14	type: {
15	15	type: 'string',
16	16	pattern: '^position$'
17	17	},
	18	+ poll: {
	19	+ $ref: '#/definitions/messageId'
	20	+ },
18	21	text: { type: 'string' },
19	22	reason: { type: 'string' },
20	23	positionDetails: {
21	24	oneOf: [

position/sync/chooseOne.jsView
		@@ -1,14 +1,15 @@
1	1	const Postion = require('./position')
2	2	const { chooseOnePositionType } = require('../../types')
3	3
4		-function ChooseOne ({ choice, body, channel, recps, mentions }) {
	4	+function ChooseOne ({ poll, choice, reason, channel, recps, mentions }) {
5	5	return Postion({
	6	+ poll,
6	7	positionDetails: {
7		- choice,
8		- type: chooseOnePositionType
	8	+ type: chooseOnePositionType,
	9	+ choice
9	10	},
10		- body,
	11	+ reason,
11	12	channel,
12	13	recps,
13	14	mentions
14	15	})

position/sync/position.jsView
		@@ -1,11 +1,9 @@
1	1	// var { link } = require('ssb-msg-schemas/util')
2	2
3		-function Position ({ positionDetails, reason, body, channel, recps, mentions }) {
4		- var content = { type: 'position', positionDetails, reason }
	3	+function Position ({ poll, positionDetails, reason, channel, recps, mentions }) {
	4	+ var content = { type: 'position', poll, positionDetails, reason }
5	5
6		- if (body) content.body = body
7		-
8	6	// if (root) {
9	7	// root = link(root)
10	8	// if (!root) { throw new Error('root is not a valid link') }
11	9	// content.root = root

test/position/sync/chooseOne.test.jsView
		@@ -2,16 +2,30 @@
2	2	const ChooseOne = require('../../../position/sync/chooseOne')
3	3	const isPosition = require('../../../isPosition')
4	4
5	5	test('Position - ChooseOne', function (t) {
6		- var invalidPosition = ChooseOne({
	6	+ var missingChoice = ChooseOne({
	7	+ poll: '%t+PhrNxxXkw/jMo6mnwUWfFjJapoPWxzsQoe0Np+nYw=.sha256'
7	8	})
8		- t.false(isPosition(invalidPosition), 'missing a choice')
	9	+ t.false(isPosition(missingChoice), 'missing a choice')
9	10
	11	+ var missingPoll = ChooseOne({
	12	+ choice: 0
	13	+ })
	14	+ t.false(isPosition(missingPoll), 'missing a poll')
	15	+
	16	+ var brokenPoll = ChooseOne({
	17	+ poll: 'dog?',
	18	+ choice: 0
	19	+ })
	20	+ t.false(isPosition(missingPoll), 'does not reference a poll')
	21	+
10	22	var validPosition = ChooseOne({
	23	+ poll: '%t+PhrNxxXkw/jMo6mnwUWfFjJapoPWxzsQoe0Np+nYw=.sha256',
11	24	choice: 0
12	25	})
13	26	t.true(isPosition(validPosition), 'simple')
	27	+ if (isPosition.errors) console.log(isPosition.errors)
14	28
15	29	var fullPositionMsg = {
16	30	key: '%somekey',
17	31	value: {

test/position/sync/isPosition.test.jsView
		@@ -4,11 +4,21 @@
4	4	// this is for testing the attributes that are required for all polls
5	5	test('position - common requirements', function (t) {
6	6	var missingDetails = {
7	7	type: 'position',
	8	+ poll: '%t+PhrNxxXkw/jMo6mnwUWfFjJapoPWxzsQoe0Np+nYw=.sha256',
8	9	positionDetails: undefined
9	10	}
10	11	t.false(isPosition(missingDetails), 'needs details')
11	12	t.true(isPosition.errors, 'failing validations have errors')
12	13
	14	+ var missingPoll = {
	15	+ type: 'position',
	16	+ positionDetails: {
	17	+ type: 'chooseOnePosition',
	18	+ choice: 0
	19	+ }
	20	+ }
	21	+ t.false(isPosition(missingPoll), 'needs poll')
	22	+
13	23	t.end()
14	24	})

types.jsView
		@@ -1,4 +1,24 @@
1	1	module.exports = {
2	2	chooseOnePollType: 'chooseOnePoll',
3	3	chooseOnePositionType: 'chooseOnePosition'
4	4	}
	5	+
	6	+// Question: do these need to be different, could we just have 'chooseOne',
	7	+// because we already have:
	8	+// { type: 'poll', pollDetails: { type: 'chooseOne' } }
	9	+// { type: 'position', positionDetails: { type: 'chooseOne' } }
	10	+//
	11	+//
	12	+// to that end, perhaps we could prune these down to
	13	+// { type: 'poll', details: { type: 'chooseOne', choices } }
	14	+// { type: 'position', details: { type: 'chooseOne', choice } }
	15	+
	16	+/// positions need to point at a poll right?
	17	+// I wonder if a position could just be ... but this would mean we would have to have the parent poll, and then based o nthe type there, run a isOneChoicePostion validator... which I wthink could be good? Maybe I don't understandthe complexity of the other types of positions.
	18	+// { type: 'position', poll, choice }
	19	+//
	20	+//
	21	+// I not that there's also a validation thing where
	22	+// { type: 'position', poll, choice: 9000 }
	23	+//
	24	+// is probably invalid, and that the choice should be an integer bound by the parent poll choices. Can probably think about that case later, but making out reducer we're gonna have to put some filtering logic somewhere. Could programatically generate a position schema based on a poll.... _WOAH there EASY MIX_

Piet / ssb-loomio