Skip to content

Conversation

@loreina
Copy link
Member

@loreina loreina commented Jul 30, 2020

List of changes:

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • New release
  • This change requires a documentation update

Steps from here:

  • 1. Use mongodump locally as backup

- [ ] 2. Add npm run seed to script run in Heroku deployment in Procfile

this means on push to master, npm run seed will be part of CI and automatically run

^ saving this for later, after the API refactor

looks like Heroku recommends processes in Procfile to only run 1 command, see: https://help.heroku.com/CTFS2TJK/how-do-i-run-multiple-processes-on-a-dyno

  • 3. Manually run npm run seed on production db

  • 4. Manually update all rolebindings in production db

Run this for each rolebinding, switching the new one in to the old one

db.rolebindings.updateMany(
    { "roles": <new> }, 
    { "$set": { "roles.$": <old> } }
)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Listed change(s) in the Changelog
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged and published in downstream modules

pierreTklein and others added 2 commits July 26, 2020 17:36
* fix: hasOwnProperty vulnerability (see https://eslint.org/docs/rules/no-prototype-builtins)
* feat: _id value to all route objects, starting at 100 and moving upwards. Add warning at top of file to maintain naming standard.
* feat: update role.constant.js to use the new _id attribute so that ids are deterministic between two runs of npm run seed.
* feat: update architecture documentation
* feat: Add HACKER_UPDATE_BATCH success message

* feat: Add promise.allsettled so that we can wait for many promises to complete

* feat: Add findByHackerId function to Account

* feat: Add updatedHackerBatch controller function, generalize validation function, fix router

* fix: resolve bug where requests hang when response is null

* feat: set route apiVersion number to be 3.0.0

* feat: create parseAcceptBatch

Co-authored-by: Pierre Theo Klein <pierre.klein@mail.mcgill.ca>
@loreina loreina requested review from logan-r and pierreTklein July 30, 2020 19:58
Copy link
Member

@logan-r logan-r left a comment

Choose a reason for hiding this comment

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

Looks good! We can automatically generate the update statements to reconcile role _ids using something like

/**
 * Create an object that defines the mapping between role `_id`s pre and post `npm run seed`
 * @param {Array} previousRoleCollection JSON export of roles collection, pre `npm run seed`
 * @param {Array} newRoleCollection JSON export of roles collection, post `npm run seed`
 */
function createRoleMigrationMapping(previousRoleCollection, newRoleCollection) {
    // `_id` mapping, keyed by role name
    let roleMigrationMapping = {}
    
    // Iterate through each role, and find their individual `_id` mapping
    for (let prevRole of previousRoleCollection) {
        roleMigrationMapping[prevRole.name] = { 
            formerId: prevRole._id.$oid,
            newId: newRoleCollection.find(newRole => newRole.name === prevRole.name)._id.$oid
        }
    }

    return roleMigrationMapping
}

// Create migration mapping - put JSON exports of roles collection here
let roleIdMapping = createRoleMigrationMapping(previousRoleCollection, newRoleCollection)

// For each role generate an updateMany statement to reconcile rolebindings
for (let role in roleIdMapping) {
    console.log(`
        db.rolebindings.updateMany(
            { "roles": ObjectId("${roleIdMapping[role].formerId}") }, 
            { "$set": { "roles.$": ObjectId("${roleIdMapping[role].newId}") } }
        )
    `)
}

@pierreTklein
Copy link
Member

pierreTklein commented Jul 30, 2020

Can you confirm that role names are unique?

Edit: Just checked: yes they are unique

@loreina loreina merged commit 51a70f9 into master Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants