Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

馃帹 Separate invites from user #7422

Merged
merged 1 commit into from
Sep 26, 2016
Merged

馃帹 Separate invites from user #7422

merged 1 commit into from
Sep 26, 2016

Conversation

kirrg001
Copy link
Contributor

@kirrg001 kirrg001 commented Sep 21, 2016 •
edited
Loading

refs #7420

With this PR we split invites from the user model 馃憤
See more in the written issue.

There is one TODO (see https://github.com/TryGhost/Ghost/pull/7422/files#diff-5da9649b589920bc040270ac06508495R88), but we can fix later.

@kevinansfield kevinansfield mentioned this pull request Sep 22, 2016
Merged
5 tasks
kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this pull request Sep 22, 2016
@kevinansfield
move to a separate Invite model
c3566a4 
issue TryGhost/Ghost#7420, requires TryGhost/Ghost#7422
- adds a new `Invite` model with associated serializer and test setup
- updates team screen to use invites rather than existing users with the "invited" property
- updates signup process to work with new invite model
- updates setup process to create invites instead of users
kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this pull request Sep 22, 2016
@kevinansfield
move to a separate Invite model
88abd3a 
issue TryGhost/Ghost#7420, requires TryGhost/Ghost#7422
- adds a new `Invite` model with associated serializer and test setup
- updates team screen to use invites rather than existing users with the "invited" property
- updates signup process to work with new invite model
- updates setup process to create invites instead of users
@kirrg001 kirrg001 force-pushed the feature/users-invites-separation branch 3 times, most recently from 15c689e to 4ce8069 Compare September 22, 2016 13:04
@kirrg001 kirrg001 changed the title [WIP] 馃帹 Separate invites from user 馃帹 Separate invites from user Sep 22, 2016
@kirrg001
Copy link
Contributor Author

ready for review 馃憤

kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this pull request Sep 23, 2016
@kevinansfield
move to a separate Invite model
476cdf8 
issue TryGhost/Ghost#7420, requires TryGhost/Ghost#7422
- adds a new `Invite` model with associated serializer and test setup
- updates team screen to use invites rather than existing users with the "invited" property
- updates signup process to work with new invite model
- updates setup process to create invites instead of users
- swaps usage of `gh-select-native` for `one-way-select` in the invite modal so that attributes can be set on the `select` element
- updates resend invite process to account for server returning a new model
- rewrites the invite management tests and fixes mirage mocks for invite endpoints
kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this pull request Sep 23, 2016
@kevinansfield
move to a separate Invite model
4327433 
issue TryGhost/Ghost#7420, requires TryGhost/Ghost#7422
- adds a new `Invite` model with associated serializer and test setup
- updates team screen to use invites rather than existing users with the "invited" property
- updates signup process to work with new invite model
- updates setup process to create invites instead of users
- swaps usage of `gh-select-native` for `one-way-select` in the invite modal so that attributes can be set on the `select` element
- updates resend invite process to account for server returning a new model
- rewrites the invite management tests and fixes mirage mocks for invite endpoints
kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this pull request Sep 23, 2016
@kevinansfield
move to a separate Invite model
e115bbd 
issue TryGhost/Ghost#7420, requires TryGhost/Ghost#7422
- adds a new `Invite` model with associated serializer and test setup
- updates team screen to use invites rather than existing users with the "invited" property
- updates signup process to work with new invite model
- updates setup process to create invites instead of users
- swaps usage of `gh-select-native` for `one-way-select` in the invite modal so that attributes can be set on the `select` element
- updates resend invite process to account for server returning a new model
- rewrites the invite management tests and fixes mirage mocks for invite endpoints
- sorts invites by email address to avoid jumping invites when re-sending
ErisDS
ErisDS reviewed Sep 25, 2016
View reviewed changes
Copy link
Member

@ErisDS ErisDS left a comment

Choose a reason for hiding this comment

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

Got a few comments, all pretty minor. Biggest thing to decide is whether the invites_roles table is necessary. This could be changed later, however it seems to add unnecessary complexity in places so thought I'd flag it up as a possible simplification.

core/server/api/authentication.js
@@ -8,6 +9,7 @@ var _ = require('lodash'),
globalUtils = require('../utils'),
utils = require('./utils'),
errors = require('../errors'),
models = require('../models'),

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

core/server/api/users.js
return pipeline(tasks, object, options);
},

/**

This comment was marked as abuse.

Sign in to view

core/server/data/migration/fixtures/fixtures.json
{
"name": "Delete invites",
"action_type": "destroy",
"object_type": "invite"

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

core/server/data/schema/schema.js
invites_roles: {
id: {type: 'increments', nullable: false, primary: true},
role_id: {type: 'integer', nullable: false},
invite_id: {type: 'integer', nullable: false}

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

core/server/models/base/index.js
@@ -388,6 +388,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
findOne: function findOne(data, options) {
data = this.filterData(data);
options = this.filterOptions(options, 'findOne');

This comment was marked as abuse.

Sign in to view

core/server/models/user.js
@@ -273,7 +269,7 @@ User = ghostBookshelf.Model.extend({

options = options || {};
optInc = options.include;
options.withRelated = _.union(options.withRelated, options.include);
options.withRelated = _.union(options.withRelated, options.include);

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

core/server/models/user.js
@@ -545,11 +538,7 @@ User = ghostBookshelf.Model.extend({
if (!user) {
return Promise.reject(new errors.NotFoundError(i18n.t('errors.models.user.noUserWithEnteredEmailAddr')));
}
if (user.get('status') === 'invited' || user.get('status') === 'invited-pending' ||

This comment was marked as abuse.

Sign in to view

@ErisDS
Copy link
Member

ErisDS commented Sep 25, 2016

P.S. I've tested and all the behaviour is working as far as I can tell 馃憤

@kirrg001
馃帹 Separate invites from user
b79a18c 
refs TryGhost#7420
- remove invite logic from user
- add invite model and adapt affected logic for inviting team members
@kirrg001 kirrg001 force-pushed the feature/users-invites-separation branch from 4ce8069 to b79a18c Compare September 26, 2016 09:08
@kirrg001
Copy link
Contributor Author

Updated the PR, waiting for travis to finish 馃憤

@kirrg001
Copy link
Contributor Author

Travis is green, ready to merge

@ErisDS ErisDS merged commit 6c24084 into TryGhost:master Sep 26, 2016
kirrg001 pushed a commit to TryGhost/Admin that referenced this pull request Sep 26, 2016
@kevinansfield @kirrg001
Separate invites and users (#277)
5913827 
closes TryGhost/Ghost#7420, requires TryGhost/Ghost#7422
- adds a new `Invite` model with associated serializer and test setup
- updates team screen to use invites rather than existing users with the "invited" property
- updates signup process to work with new invite model
- updates setup process to create invites instead of users
- swaps usage of `gh-select-native` for `one-way-select` in the invite modal so that attributes can be set on the `select` element
- updates resend invite process to account for server returning a new model
- rewrites the invite management tests and fixes mirage mocks for invite endpoints
- sorts invites by email address to avoid jumping invites when re-sending
@ErisDS ErisDS removed their assignment Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinansfield kevinansfield kevinansfield left review comments

@ErisDS ErisDS ErisDS left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kirrg001 @ErisDS @kevinansfield