918952 - Add makeid to Goggles project model

Status: RESOLVED FIXED

(Webmaker Graveyard :: X-Ray Goggles, defect)

Description

[Matthew Schranz [:mjschranz]]

We are going to need goggles projects saving the makeid for cascading deletes.

Comment 1

[Matthew Schranz [:mjschranz]]

Attachment: https://github.com/mozilla/goggles.webmaker.org/pull/28

I'm not very familiar with Goggles, but as best I can tell it's nearly identical to Thimble as far as it's publishing system goes.

Comment 2

[Pomax]

Comment on attachment 807909 [details] [review]
https://github.com/mozilla/goggles.webmaker.org/pull/28

in fact, the code you touche here is identical between the two projects. r+

Comment 3

[Jon Buckley [:jbuck]]

Comment on attachment 807909 [details] [review]
https://github.com/mozilla/goggles.webmaker.org/pull/28

r- with some nits noted in the PR. You'll need to add the new column to lib/models/thimbleproject.js, add the unique constraint to it first.

Comment 4

[Matthew Schranz [:mjschranz]]

Comment on attachment 807909 [details] [review]
https://github.com/mozilla/goggles.webmaker.org/pull/28

Fixed.

Are you really sure about https://github.com/mozilla/goggles.webmaker.org/pull/28/files#r6510335? Seems to conflict with the recent problems we had with MakeAPI and contentType, although that was Mongo and not SQL.

Comment 5

[Jon Buckley [:jbuck]]

Comment on attachment 807909 [details] [review]
https://github.com/mozilla/goggles.webmaker.org/pull/28

r-:

* add the unique constraint to the Sequelize model
* I got a crash on lib/middleware.js:189:

ReferenceError: project is not defined
    at /Users/jon/Sites/goggles.webmaker.org/lib/middleware.js:189:25
    at null.<anonymous> (/Users/jon/Sites/goggles.webmaker.org/lib/database.js:115:37)
    at EventEmitter.emit (events.js:95:17)
    at null.<anonymous> (/Users/jon/Sites/goggles.webmaker.org/node_modules/sequelize/lib/query-interface.js:291:17)
    at EventEmitter.emit (events.js:117:20)
    at null.<anonymous> (/Users/jon/Sites/goggles.webmaker.org/node_modules/sequelize/lib/dialects/mysql/query.js:32:14)
    at Query.Sequence.end (/Users/jon/Sites/goggles.webmaker.org/node_modules/mysql/lib/protocol/sequences/Sequence.js:66:24)
    at Query._handleFinalResultPacket (/Users/jon/Sites/goggles.webmaker.org/node_modules/mysql/lib/protocol/sequences/Query.js:139:8)
    at Query.OkPacket (/Users/jon/Sites/goggles.webmaker.org/node_modules/mysql/lib/protocol/sequences/Query.js:73:10)
    at Protocol._parsePacket (/Users/jon/Sites/goggles.webmaker.org/node_modules/mysql/lib/protocol/Protocol.js:169:24)

Comment 6

[Matthew Schranz [:mjschranz]]

Comment on attachment 807909 [details] [review]
https://github.com/mozilla/goggles.webmaker.org/pull/28

Oops.

Comment 7

[Jon Buckley [:jbuck]]

Comment on attachment 807909 [details] [review]
https://github.com/mozilla/goggles.webmaker.org/pull/28

r+ with nits, add "async" to the package.json file and this is ready to roll!

Comment 8

[Jon Buckley [:jbuck]]

Matt, merge at will, but don't push to staging until :jp has changed the DB on staging okay?

Comment 9

[Matthew Schranz [:mjschranz]]

Landed as https://github.com/mozilla/goggles.webmaker.org/commit/708817078b7d61574c9b380b3dd96625858d506a