Upgrade discussions, improve documentation

apps/discussions/README.md

@@ -2,24 +2,18 @@
 
 This repository is the starting point of contextual aragon discussions, it's composed of a few core components that a developer wishing to incorporate discussions needs to be aware of:
 
-1. Contextual discussion smart contract - in charge of storing all the DAO's discussion data. Each discussion post is represented as an IPFS content hash to keep storage costs as efficient as possible.
-2. `DiscussionsWrapper` component - a redux-like provider that provides discussion data through React context to all nested children and grandchildren.
-3. `Discussion` component - a discussion thread component that displays all the discussion posts of a specific discussion thread and allows the user to take specific actions like post, hide, and revise.
+1. Contextual discussion [smart contract](https://github.com/AutarkLabs/open-enterprise/blob/dev/apps/discussions/contracts/DiscussionApp.sol) - in charge of storing all the DAO's discussion data. Each discussion post is represented as an IPFS content hash to keep storage costs as efficient as possible.
+2. [`Discussions` component](https://github.com/AutarkLabs/open-enterprise/blob/dev/apps/discussions/app/modules/Discussions.js) - a redux-like provider that provides discussion data through React context to all nested children and grandchildren.
+3. [`Discussion` component](https://github.com/AutarkLabs/open-enterprise/blob/dev/apps/discussions/app/modules/Discussion.js) - a discussion thread component that displays all the discussion posts of a specific discussion thread and allows the user to take specific actions like post, hide, and revise.
 
 The purpose of this readme is to document how all the moving parts are working together, how a developer could use this code in their own DAO, and what still needs to be done.
 
 ### Prerequisites
 
-You first need to be running your contextual discussioned app + [aragon client](https://github.com/aragon/aragon) with a very specific version of aragon.js. You will need a version with the following 3 features:
-
-1. [External transaction intents](https://github.com/aragon/aragon.js/pull/328)
-2. [Ability to get information about the DAO's installed apps](https://github.com/aragon/aragon.js/pull/332)
-3. [New forwarder API changes](https://github.com/aragon/aragon.js/pull/314)
+You first need to be running your contextual discussioned app + [aragon client](https://github.com/aragon/aragon) with a very specific version of aragon.js. You will need a version with the new [forwarder API changes](https://github.com/aragon/aragon.js/pull/314).
 
 We'd recommend running the latest master branch of the [aragon client](https://github.com/aragon/aragon).
 
-These features should be included by default in aragon.js and aragon client come October 2019.
-
 ### Setup
 
 ##### Including the discussions app in your repo
@@ -41,7 +35,7 @@ function createDiscussionApp(address root, ACL acl, Kernel dao) internal returns
 
 ##### Setting up the discussions app as a forwarder
 
-Every action that gets forwarded through the discussions app creates a new discussion thread. So we need to add the discussions app to the forwarding chain of any action we want to trigger a discussion. In this example, we have a new discussion created _before_ a new dot-vote gets created:
+Every action that gets forwarded through the discussions app creates a new discussion thread. So we need to add the discussions app to the forwarding chain of any action we want to trigger a discussion. This is so developers do not have to manage creating their own discussion thread IDs, but this decision might change (more in the section on "forwarded actions" below). In this example, we have a new discussion created _before_ a new dot-vote gets created:
 
 ```
 acl.createPermission(discussions, dotVoting, dotVoting.ROLE_CREATE_VOTES(), voting);
@@ -91,7 +85,6 @@ The `discussionId` is a `Number` that represents the relative order in which thi
 
 1, 2, 3, 4, 5 or 14, 15, 16, 19, 20. The only thing that matters is the _order_ the transactions occured. The discussion app will figure out the rest for you.
 
-
 ##### How this all works under the hood
 
 The discussions app generates a new discussion thread every time an action gets successfully forwarded. When the discussions app script loads, it uses the latest [forwarder api](https://github.com/aragon/aragon.js/pull/314) to [keep track of which discussion threads belong to which app](https://github.com/AutarkLabs/planning-suite/blob/discussions/apps/discussions/app/script.js#L36).
@@ -100,3 +93,8 @@ On the frontend, the `Discussions.js` component senses when the handshake has be
 
 The discussionApi is responsible for [keeping track of all the discussion threads and posts for your app](https://github.com/AutarkLabs/planning-suite/blob/discussions/apps/discussions/app/modules/DiscussionsApi.js#L136). Its also equipped with methods to [post](https://github.com/AutarkLabs/planning-suite/blob/discussions/apps/discussions/app/modules/DiscussionsApi.js#L162), [hide](https://github.com/AutarkLabs/planning-suite/blob/discussions/apps/discussions/app/modules/DiscussionsApi.js#L197), and [revise](https://github.com/AutarkLabs/planning-suite/blob/discussions/apps/discussions/app/modules/DiscussionsApi.js#L175) Discussion Posts.
 
+###### Changing the discussionId paradigm
+
+Thanks to [Chad Ostrowski](https://github.com/chadoh/) for this idea - instead of creating new discussions as a step in a forwarder chain, developers should be able to "create a dicsussion thread" on their own, and completely manage their ID schema in whatever way they choose.
+
+This would require some under the hood shifts, but should be possible. To handle this feature change, the discussions app should prepend the app identifier to the ID before creating a discussion thread to avoid namespace conflicts: `[appIdentifier][discussionId]`.

apps/discussions/app/modules/Comment.js

@@ -1,4 +1,4 @@
-import React, { useState } from 'react'
+import React, { useState, useEffect, Fragment } from 'react'
 import PropTypes from 'prop-types'
 import styled from 'styled-components'
 import { format, formatDistance } from 'date-fns'
@@ -10,6 +10,7 @@ const Header = styled.header`
   display: flex;
   justify-content: space-between;
   margin-bottom: 10px;
+  width: 100%;
 `
 
 const TimeAgo = styled.time.attrs(props => ({
@@ -23,11 +24,15 @@ TimeAgo.propTypes = {
   date: PropTypes.instanceOf(Date).isRequired,
 }
 
-const Top = ({ author, createdAt }) => {
+const Top = ({ author, createdAt, identity }) => {
   const created = new Date(Number(createdAt) * 1000)
   return (
     <Header>
-      <IdentityBadge entity={author} />
+      {identity && identity.name ? (
+        <IdentityBadge entity={author} customLabel={identity.name} />
+      ) : (
+        <IdentityBadge entity={author} />
+      )}
       <TimeAgo date={created} />
     </Header>
   )
@@ -112,18 +117,32 @@ const Bottom = ({ onDelete, onEdit }) => {
 }
 
 const Comment = ({
+  app,
   currentUser,
   comment: { author, id, text, createdAt, revisions, postCid },
   onDelete,
   onSave,
 }) => {
   const [editing, setEditing] = useState(false)
+  const [identity, setIdentity] = useState(null)
 
   const update = async updated => {
     await onSave({ id, text: updated.text, revisions, postCid })
     setEditing(false)
   }
 
+  useEffect(() => {
+    const resolveIdentity = async () => {
+      const addressIdentity = await app
+        .resolveAddressIdentity(author)
+        .toPromise()
+      if (addressIdentity) {
+        setIdentity(addressIdentity)
+      }
+    }
+    resolveIdentity()
+  }, [author])
+
   return (
     <CommentCard>
       {editing ? (
@@ -133,13 +152,13 @@ const Comment = ({
           onSave={update}
         />
       ) : (
-        <React.Fragment>
-          <Top author={author} createdAt={createdAt} />
+        <Fragment>
+          <Top author={author} identity={identity} createdAt={createdAt} />
           <Markdown content={text} />
           {author === currentUser && (
             <Bottom onDelete={onDelete} onEdit={() => setEditing(true)} />
           )}
-        </React.Fragment>
+        </Fragment>
       )}
     </CommentCard>
   )

apps/discussions/app/modules/Discussion.js

@@ -5,7 +5,7 @@ import Comment from './Comment'
 import CommentForm from './CommentForm'
 
 const Discussion = ({ discussionId, ethereumAddress }) => {
-  const { discussion, discussionApi } = useDiscussion(discussionId)
+  const { app, discussion, discussionApi } = useDiscussion(discussionId)
 
   const save = ({ text, id, revisions, postCid }) =>
     id
@@ -29,6 +29,7 @@ const Discussion = ({ discussionId, ethereumAddress }) => {
     <div css="margin-bottom: 40px">
       {discussion.map(comment => (
         <Comment
+          app={app}
           comment={comment}
           currentUser={ethereumAddress}
           key={comment.id}

apps/discussions/app/modules/Discussions.js

@@ -26,9 +26,9 @@ const Discussions = ({ children, app }) => {
     if (!hasInit && handshakeOccured) {
       initDiscussions()
     }
-  }, [handshakeOccured])
+  }, [hasInit, handshakeOccured])
   return (
-    <DiscussionsContext.Provider value={{ discussions, discussionApi }}>
+    <DiscussionsContext.Provider value={{ app, discussions, discussionApi }}>
       {children}
     </DiscussionsContext.Provider>
   )

apps/discussions/app/modules/DiscussionsApi.js

@@ -1,3 +1,16 @@
+/*
+ * The DiscussionsApi is a mock-backend service that implements functionality
+ * for interacting with contextual discussions app.
+ *
+ * This backend system is meant to be replaceable - so we could swap
+ * in other backend services like mongoDB, orbitDB...etc
+ *
+ * Its two responsibilities are:
+ * (1) to manage discussion states (listening for updates, collecting info)
+ * and (2) to provide a gateway to interacting with discussions
+ *
+ */
+
 import cloneDeep from 'lodash.clonedeep'
 import { ipfs } from '../ipfs'
 
@@ -38,11 +51,20 @@ class Discussions {
   _pastEvents = () =>
     new Promise(resolve =>
       this.contract.pastEvents().subscribe(events => {
-        this.lastEventBlock = events[events.length - 1].blockNumber
-        resolve(events)
+        if (events.length > 0) {
+          this.lastEventBlock = events[events.length - 1].blockNumber
+          return resolve(events)
+        }
+        resolve([])
       })
     )
 
+  /*
+   * Here we're using hardcoded discussion ThreadIds to get around the
+   * fact that the forwarder API changes are not yet merged. We rely on
+   * the forwarder API to generate and organize discussion thread IDs across
+   * various apps
+   */
   _collectDiscussionThreadIds = () =>
     new Promise(resolve => {
       resolve(
@@ -154,6 +176,7 @@ class Discussions {
       // })
     })
 
+  // We only want events from the api that pertain to discussions
   _filterRelevantDiscussionEvents = (discussionThreadIds, discussionEvents) => {
     return discussionEvents.filter(
       ({ event, returnValues }) =>
@@ -162,6 +185,8 @@ class Discussions {
     )
   }
 
+  /* ~~~~~ STATE MANAGEMENT ~~~~~~~ */
+
   _handleHide = async (
     state,
     { returnValues: { discussionThreadId, postId } }
@@ -236,6 +261,8 @@ class Discussions {
     return this._buildState(newState, events)
   }
 
+  /* ~~~~~~ FETCH AND LISTEN FOR INFORMATION ~~~~~~~ */
+
   collect = async () => {
     const relevantDiscussionThreads = await this._collectDiscussionThreadIds()
     const allDiscussionEvents = await this._pastEvents()
@@ -257,10 +284,14 @@ class Discussions {
   }
 
   listenForUpdates = callback =>
-    this.contract.events(this.lastEventBlock + 1).subscribe(async event => {
-      this.discussions = await this._buildState(this.discussions, [event])
-      callback(this.discussions)
-    })
+    this.contract
+      .events({ fromBlock: this.lastEventBlock + 1 })
+      .subscribe(async event => {
+        this.discussions = await this._buildState(this.discussions, [event])
+        callback(this.discussions)
+      })
+
+  /* ~~~~~ INTERACTIONS WITH THE DISCUSSIONS SMART CONTRACT ~~~~~~~ */
 
   post = async (text, discussionThreadId, ethereumAddress) => {
     const discussionPost = {

apps/discussions/app/modules/getDiscussion.js

@@ -6,7 +6,7 @@
 const getDiscussion = (relativeDiscussionId, discussions) => {
   const absoluteDiscussionId = Object.keys(discussions)
     .map(discussionId => Number(discussionId))
-    .sort()[relativeDiscussionId]
+    .sort((a, b) => a - b)[relativeDiscussionId]
 
   return discussions[absoluteDiscussionId] || {}
 }

apps/discussions/app/modules/useDiscussion.js

@@ -2,12 +2,12 @@ import { useContext } from 'react'
 import { getDiscussion, DiscussionsContext } from './'
 
 const useDiscussion = id => {
-  const { discussions, discussionApi } = useContext(DiscussionsContext)
+  const { app, discussions, discussionApi } = useContext(DiscussionsContext)
   const discussionObj = getDiscussion(id, discussions)
   const discussionArr = Object.keys(discussionObj)
     .sort((a, b) => discussionObj[a].createdAt - discussionObj[b].createdAt)
     .map(postId => ({ ...discussionObj[postId], id: postId }))
-  return { discussion: discussionArr, discussionApi }
+  return { app, discussion: discussionArr, discussionApi }
 }
 
 export default useDiscussion

apps/discussions/app/modules/useHandshake.js

@@ -1,19 +1,13 @@
 import { useEffect, useState } from 'react'
 
+/*
+ * A function that tells us when it's safe to start loading discussion
+ * data from the aragon/api
+ * */
+
 export default () => {
   const [handshakeOccured, setHandshakeOccured] = useState(false)
-  const sendMessageToWrapper = (name, value) => {
-    window.parent.postMessage({ from: 'app', name, value }, '*')
-  }
-  const handleWrapperMessage = ({ data }) => {
-    if (data.from !== 'wrapper') {
-      return
-    }
-    if (data.name === 'ready') {
-      sendMessageToWrapper('ready', true)
-      setHandshakeOccured(true)
-    }
-  }
+  const handleWrapperMessage = ({ data }) => setHandshakeOccured(true)
   useEffect(() => {
     return window.addEventListener('message', handleWrapperMessage)
   }, [])

apps/discussions/arapp.json

@@ -7,6 +7,12 @@
     }
   ],
   "environments": {
+    "staging": {
+      "registry": "0x98Df287B6C145399Aaa709692c8D308357bC085D",
+      "appName": "discussions-staging.open.aragonpm.eth",
+      "wsRPC": "wss://rinkeby.eth.aragon.network/ws",
+      "network": "rinkeby"
+    },
     "default": {
       "registry": "0x5f6f7e8cc7346a11ca2def8f827b7a0b612c56a1",
       "network": "rpc",

apps/discussions/package.json

@@ -3,17 +3,19 @@
   "version": "1.0.0",
   "description": "",
   "dependencies": {
-    "@aragon/api": "2.0.0-beta.8",
-    "@aragon/api-react": "2.0.0-beta.6",
-    "@aragon/ui": "1.0.0-alpha.11",
     "@babel/polyfill": "^7.2.5",
+    "ipfs-http-client": "^32.0.1",
+    "lodash.clonedeep": "^4.5.0",
+    "react-markdown": "^4.1.0",
     "prop-types": "^15.7.2",
+    "styled-components": "4.1.3"
+  },
+  "peerDependencies": {
+    "@aragon/api": "2.0.0-beta.8",
+    "@aragon/api-react": "2.0.0-beta.6",
+    "@aragon/ui": "^1.0.0",
     "react": "^16.8.6",
-    "react-dom": "^16.8.6",
-    "react-markdown": "^4.1.0",
-    "styled-components": "4.1.3",
-    "ipfs-http-client": "^32.0.1",
-    "lodash.clonedeep": "^4.5.0"
+    "react-dom": "^16.8.6"
   },
   "devDependencies": {
     "@babel/core": "^7.4.5",
@@ -50,10 +52,10 @@
     "start:app": "cd app && npm start && cd ..",
     "test": "cross-env TRUFFLE_TEST=true npm run ganache-cli:test",
     "compile": "aragon contracts compile",
-    "build": "npm run sync-assets && npm run build:app && npm run build:script",
+    "build": "npm run compile && npm run sync-assets && npm run build:app && npm run build:script",
     "build:app": "parcel build ./app/index.html -d dist/ --public-url \".\" --no-cache",
     "build:script": "parcel build ./app/script.js -d dist/ --no-cache",
-    "publish:cd": "../../shared/deployments/check-publish.sh",
+    "publish:cd": "yes | aragon apm publish major --files dist/ --environment continuous-deployment --apm.ipfs.rpc https://ipfs.autark.xyz:5001 --ipfs-check false --skip-confirmation true",
     "publish:http": "npm run build:script && yes | aragon apm publish major --files dist --http localhost:9999 --http-served-from ./dist --propagate-content false --skip-confirmation true",
     "publish:patch": "aragon apm publish patch",
     "publish:minor": "aragon apm publish minor",
@@ -62,7 +64,8 @@
     "lint": "solium --dir ./contracts",
     "lint:fix": "eslint . --fix && solium --dir ./contracts --fix",
     "coverage": "cross-env SOLIDITY_COVERAGE=true npm run ganache-cli:test",
-    "ganache-cli:test": "sh ./node_modules/@aragon/test-helpers/ganache-cli.sh"
+    "ganache-cli:test": "sh ./node_modules/@aragon/test-helpers/ganache-cli.sh",
+    "frontend": "npm run sync-assets && parcel app/index.html --port 9999"
   },
   "browserslist": [
     "last 2 Chrome versions"