Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

[WIP][GH-4359] fix wrong email notification after leaving review #4426

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions infra/notifications/templates/messageTemplates.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ const messageTemplates = {
body: template(
'Your transaction for <%= listing.title %> has been completed.'
)
},
OfferData: {
title: template('New Review for <%= listing.title %>'),
body: template(
'A review has been left on your transaction for <%= listing.title %>.'
)
}
},
email: {
Expand Down Expand Up @@ -127,6 +133,15 @@ const messageTemplates = {
text: template(
fs.readFileSync(`${templateDir}/seller-OfferFinalized.txt`).toString()
)
},
OfferData: {
Copy link
Contributor

Choose a reason for hiding this comment

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

We have OfferFinalized, OfferCreated, ...Offer[Action]. It would be nice to name this accordingly OfferReviewed or something to that nature.

I would like an Origin team member to weigh in on this before you make any changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OfferData is event name from our Solidity contracts. Don't think we plan on upgrading the contracts anytime soon. So, I'd suggest to leave it as it is for now.

subject: template('New Review for <%= listing.title %>'),
mjml: template(
fs.readFileSync(`${templateDir}/buyer-OfferReview.mjml`).toString()
Copy link
Contributor

Choose a reason for hiding this comment

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

These files should get renamed to the seller side

Suggested change
fs.readFileSync(`${templateDir}/buyer-OfferReview.mjml`).toString()
fs.readFileSync(`${templateDir}/seller-OfferReview.mjml`).toString()

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that's reasonable. I'd suggest to change it too.

Copy link
Contributor

@franckc franckc Apr 17, 2020

Choose a reason for hiding this comment

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

Unless I'm mistaken, I don't think the template files seller-OfferReview.mjml/txt already exist?
They should be added as part of this PR.

),
text: template(
fs.readFileSync(`${templateDir}/buyer-OfferReview.txt`).toString()
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Suggested change
fs.readFileSync(`${templateDir}/buyer-OfferReview.txt`).toString()
fs.readFileSync(`${templateDir}/seller-OfferReview.txt`).toString()

)
}
}
},
Expand Down Expand Up @@ -155,12 +170,6 @@ const messageTemplates = {
body: template(
'A ruling has been issued on your disputed transaction for <%= listing.title %>.'
)
},
OfferData: {
title: template('New Review for <%= listing.title %>'),
body: template(
'A review has been left on your transaction for <%= listing.title %>.'
)
}
},
email: {
Expand Down Expand Up @@ -199,15 +208,6 @@ const messageTemplates = {
text: template(
fs.readFileSync(`${templateDir}/buyer-OfferRuling.txt`).toString()
)
},
OfferData: {
subject: template('New Review for <%= listing.title %>'),
mjml: template(
fs.readFileSync(`${templateDir}/buyer-OfferReview.mjml`).toString()
),
text: template(
fs.readFileSync(`${templateDir}/buyer-OfferReview.txt`).toString()
)
}
}
}
Expand Down
299 changes: 299 additions & 0 deletions infra/notifications/test/fixtures/OfferData.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,299 @@
{
"event": {
"blockHash": "0x30fed0ffa346133488faa3b9760d78f909502d586c6e41b414bd9eff77adea2a",
"transactionHash": "0x34317598fc148b22879b3986911e26977ef91742eda64883d9b434417931b45f",
"blockNumber": 4181146,
"address": "0xeBf3C3b116fC39b05a4B6E24aCB3C7c5e604d07B",
"logIndex": 1,
"removed": false,
"transactionIndex": 2,
"id": "log_a4b73925",
"returnValues": {
"0": "0x627306090abaB3A6e1400e9345bC60c78a8BEf57",
"1": "0",
"2": "1",
"3": "0xe0f6faedd9d5dd8e6638bb1cd2a2d8de6f99cffa928e872ab4ef7258f17fa157",
"party": "0x627306090abaB3A6e1400e9345bC60c78a8BEf57",
"listingID": "0",
"offerID": "1",
"ipfsHash": "0xe0f6faedd9d5dd8e6638bb1cd2a2d8de6f99cffa928e872ab4ef7258f17fa157"
},
"event": "OfferData",
"signature": "0x9b7312a236066d2da679eba7e3a2e86d2d07a973819499846c0efd2fd2506c80",
"raw": {
"data": "0xe0f6faedd9d5dd8e6638bb1cd2a2d8de6f99cffa928e872ab4ef7258f17fa157",
"topics": [
"0x449224201a35688b74a38ff24770e8b2a326ebf42357bf19a36f5fedbbe552a0",
"0x000000000000000000000000f17f52151ebef6c7334fad080c5704d77216b732",
"0x000000000000000000000000000000000000000000000000000000000000020a",
"0x0000000000000000000000000000000000000000000000000000000000000002"
]
},
"block": {
"id": 4181146
}
},
"related": {
"offer": {
"listing": {
"id": "1-000-522-4181140",
"valid": true,
"validationError": null,
"status": "pending",
"totalEvents": 8,
"seller": {
"id": "0xf17f52151EbEF6C7334FAD080c5704D77216b732",
"identity": {
"firstName": "Tom",
"lastName": "Linton",
"fullName": "Tom Linton",
"owner": {
"id": "0xf17f52151EbEF6C7334FAD080c5704D77216b732",
"proxy": {
"id": "0xf17f52151EbEF6C7334FAD080c5704D77216b732"
}
},
"__typename": "Identity"
},
"__typename": "Account"
},
"arbitrator": {
"id": "0xf17f52151EbEF6C7334FAD080c5704D77216b732",
"__typename": "Account"
},
"deposit": "0",
"depositAvailable": "0",
"createdEvent": {
"timestamp": 1554845133,
"__typename": "Event"
},
"category": "schema.forSale",
"categoryStr": "For Sale",
"subCategory": "schema.antiques",
"title": "Listener Test",
"description": "A listing to test event listener. ",
"currencyId": null,
"featured": false,
"hidden": false,
"price": {
"amount": "0.25",
"currency": {
"id": "fiat-USD",
"__typename": "FiatCurrency"
},
"__typename": "Price"
},
"acceptedTokens": [
{
"id": "token-DAI",
"__typename": "Token"
},
{
"id": "token-ETH",
"__typename": "Token"
}
],
"media": [
{
"url": "ipfs://QmfFFvvY2jUZaEsGyaxwtMEnsyVPpCHruLbQEkR8XhSpJq",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmVC84UfjAZwwusJU2mC2Lq3VXcTVpJzCB7V9Rujz1jELr",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmNWWqDZWC9qzRaMDX7Gq61qxhdSymNsYFGXS1QjZo1fDe",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmVdbpzoUjo3mV8QAFnh32dsDnrGVj6WnFNPsAu1JZh68X",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmW9iwKTq89kBrQx3io587QS8oUkg4H7bSzBS1oi8oa1aN",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmYAtgg2s6kHHurKneF95pyncrN8FKsDmU4tPhDDdQQAe4",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmRNa1GoiyQJeFcLXL38cLCpzJQjgyhTKnmp1Wfoo1Kgui",
"contentType": "image/jpeg"
},
{
"url": "ipfs://Qmb84TG5qWPY7nZXvzGnr3z9APB1t8aiPd8YeVooKg1mQc",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmbsGhwdba8s8FCJcvNbrPtVsigR4VoK1p65k1YgGgFT6m",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmUg8RS85PKGdgAVSDhoSRBfU19XeHaWdjEmGo7zS3McLW",
"contentType": "image/jpeg"
}
],
"commission": "0",
"commissionPerUnit": "0",
"marketplacePublisher": null,
"unitsTotal": 1,
"unitsAvailable": 0,
"unitsSold": 0,
"unitsPending": 1,
"multiUnit": false,
"__typename": "UnitListing"
},
"id": "1-000-522-2",
"listingId": "1-000-522",
"offerId": "2",
"value": "257500000000000000",
"currency": "0x2448eE2641d78CC42D7AD76498917359D961A783",
"refund": "0",
"commission": "0",
"status": 2,
"statusStr": "Accepted",
"finalizes": 1556056158,
"quantity": 1,
"valid": true,
"validationError": null,
"arbitrator": {
"id": "0xc9C1a92ba54C61045Ebf566B154dfD6AfeDEA992",
"__typename": "Account"
},
"affiliate": {
"id": "0xC1A33cdA27c68E47e370FF31CdAd7D6522Ea93d5",
"__typename": "Account"
},
"buyer": {
"id": "0x627306090abaB3A6e1400e9345bC60c78a8BEf57",
"identity": {
"firstName": "Franck",
"lastName": "Test",
"fullName": "Franck Test",
"owner": {
"id": "0x627306090abaB3A6e1400e9345bC60c78a8BEf57",
"proxy": {
"id": "0x627306090abaB3A6e1400e9345bC60c78a8BEf57"
}
},
"__typename": "Identity"
},
"__typename": "Account"
},
"withdrawnBy": null,
"__typename": "Offer"
},
"listing": {
"id": "1-000-522-4181140",
"valid": true,
"validationError": null,
"status": "pending",
"totalEvents": 8,
"seller": {
"id": "0xf17f52151EbEF6C7334FAD080c5704D77216b732",
"identity": {
"firstName": "Tom",
"lastName": "Linton",
"fullName": "Tom Linton",
"owner": {
"id": "0xf17f52151EbEF6C7334FAD080c5704D77216b732",
"proxy": {
"id": "0xf17f52151EbEF6C7334FAD080c5704D77216b732"
}
},
"__typename": "Identity"
},
"__typename": "Account"
},
"arbitrator": {
"id": "0xf17f52151EbEF6C7334FAD080c5704D77216b732",
"__typename": "Account"
},
"deposit": "0",
"depositAvailable": "0",
"createdEvent": {
"timestamp": 1554845133,
"__typename": "Event"
},
"category": "schema.forSale",
"categoryStr": "For Sale",
"subCategory": "schema.antiques",
"title": "Listener Test",
"description": "A listing to test event listener. ",
"currencyId": null,
"featured": false,
"hidden": false,
"price": {
"amount": "0.25",
"currency": {
"id": "fiat-USD",
"__typename": "FiatCurrency"
},
"__typename": "Price"
},
"acceptedTokens": [
{
"id": "token-DAI",
"__typename": "Token"
},
{
"id": "token-ETH",
"__typename": "Token"
}
],
"media": [
{
"url": "ipfs://QmfFFvvY2jUZaEsGyaxwtMEnsyVPpCHruLbQEkR8XhSpJq",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmVC84UfjAZwwusJU2mC2Lq3VXcTVpJzCB7V9Rujz1jELr",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmNWWqDZWC9qzRaMDX7Gq61qxhdSymNsYFGXS1QjZo1fDe",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmVdbpzoUjo3mV8QAFnh32dsDnrGVj6WnFNPsAu1JZh68X",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmW9iwKTq89kBrQx3io587QS8oUkg4H7bSzBS1oi8oa1aN",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmYAtgg2s6kHHurKneF95pyncrN8FKsDmU4tPhDDdQQAe4",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmRNa1GoiyQJeFcLXL38cLCpzJQjgyhTKnmp1Wfoo1Kgui",
"contentType": "image/jpeg"
},
{
"url": "ipfs://Qmb84TG5qWPY7nZXvzGnr3z9APB1t8aiPd8YeVooKg1mQc",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmbsGhwdba8s8FCJcvNbrPtVsigR4VoK1p65k1YgGgFT6m",
"contentType": "image/jpeg"
},
{
"url": "ipfs://QmUg8RS85PKGdgAVSDhoSRBfU19XeHaWdjEmGo7zS3McLW",
"contentType": "image/jpeg"
}
],
"commission": "0",
"commissionPerUnit": "0",
"marketplacePublisher": null,
"unitsTotal": 1,
"unitsAvailable": 0,
"unitsSold": 0,
"unitsPending": 1,
"multiUnit": false,
"__typename": "UnitListing"
}
}
}
20 changes: 20 additions & 0 deletions infra/notifications/test/marketplace.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const MobileRegistry = require('../src/models/index').MobileRegistry
const NotificationLog = require('../src/models').NotificationLog

const OfferAccepted = require('./fixtures/OfferAccepted.json')
const OfferData = require('./fixtures/OfferData.json')

describe('Email and MobilePush notifications for Marketplace events', () => {
before(async () => {
Expand Down Expand Up @@ -75,4 +76,23 @@ describe('Email and MobilePush notifications for Marketplace events', () => {
})
expect(logMobile).to.be.an('object')
})

it(`Should send an email and a mobile push notification for a review given by buyer`, async () => {
await request(app)
.post('/events')
.send(OfferData)
.expect(200)

// For a review given, only the seller gets notified.
// There should be 2 entries in notification_log, one for email and the
// other for the mobile push.
const logEmail = await NotificationLog.findOne({
where: { ethAddress: this.seller, channel: 'email' }
})
expect(logEmail).to.be.an('object')
const logMobile = await NotificationLog.findOne({
where: { ethAddress: this.seller, channel: 'mobile-ios' }
})
expect(logMobile).to.be.an('object')
})
})