mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-03-18 02:21:47 -05:00
Fixed member ProxyObject from sparse array leaking out of members list (#15565)
no issue
- the `ella-sparse-array` dependency used for the sparsely populated list on the members screen creates ProxyObjects that wrap the underlying member model instances meaning the forced use of `.get()` and `.set()` required by ProxyObject was leaking through to other areas of the app causing a mismatch in code patterns
- moved the loading state for each member into a separate component and put the loading conditional directly inside the `{{#each members}}` block so that we can pass the real model instance through to components via `{{member.content}}` rather than passing the ProxyObject wrapper, avoiding unexpected errors when not using `.get()` and `.set()` on member arguments
This commit is contained in:
Kevin Ansfield
GitHub
parent
7b443d4b63
commit
7eab83a6ec
9 changed files with 101 additions and 105 deletions
|
|
@ -1,5 +1,4 @@
|
|||
import Component from '@glimmer/component';
|
||||
import {get} from '@ember/object';
|
||||
import {htmlSafe} from '@ember/template';
|
||||
|
||||
const stringToHslColor = function (str, saturation, lightness) {
|
||||
|
|
@ -16,15 +15,14 @@ export default class GhMemberAvatarComponent extends Component {
|
|||
get memberName() {
|
||||
let {member} = this.args;
|
||||
|
||||
// can be given a proxy object from a sparse array so get is required
|
||||
return get(member, 'name') || get(member, 'email') || 'NM';
|
||||
return member?.name || member?.email || 'NM';
|
||||
}
|
||||
|
||||
get avatarImage() {
|
||||
let {member} = this.args;
|
||||
|
||||
// to cover both ways avatar image is returned depending on where member data comes from
|
||||
return get(member, 'avatar_image') || get(member, 'avatarImage') || null;
|
||||
return member?.avatar_image || member?.avatarImage || null;
|
||||
}
|
||||
|
||||
get backgroundStyle() {
|
||||
|
|
|
|||
12
ghost/admin/app/components/gh-members-list-item-loading.hbs
Normal file
12
ghost/admin/app/components/gh-members-list-item-loading.hbs
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
<tr data-test-list="members-list-item">
|
||||
<div class="gh-list-data gh-members-list-basic gh-list-loadingcell">
|
||||
<div class="gh-list-loading-title"></div>
|
||||
<div class="gh-list-loading-detail"></div>
|
||||
</div>
|
||||
<div class="gh-list-data"></div>
|
||||
<div class="gh-list-data"></div>
|
||||
<div class="gh-list-data"></div>
|
||||
{{#each @filterColumns}}
|
||||
<div class="gh-list-data"></div>
|
||||
{{/each}}
|
||||
</tr>
|
||||
|
|
@ -1,82 +1,69 @@
|
|||
<tr data-test-list='members-list-item' data-test-member={{@member.id}}>
|
||||
{{#if @member.is_loading}}
|
||||
<div class="gh-list-data gh-members-list-basic gh-list-loadingcell">
|
||||
<div class="gh-list-loading-title"></div>
|
||||
<div class="gh-list-loading-detail"></div>
|
||||
</div>
|
||||
<div class="gh-list-data"></div>
|
||||
<div class="gh-list-data"></div>
|
||||
<div class="gh-list-data"></div>
|
||||
{{#each @filterColumns}}
|
||||
<div class="gh-list-data"></div>
|
||||
{{/each}}
|
||||
{{else}}
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data" data-test-table-data="details">
|
||||
<div class="flex items-center">
|
||||
<GhMemberAvatar @member={{@member}} @containerClass="w9 h9 mr3 flex-shrink-0" />
|
||||
<div class="w-80">
|
||||
<h3 class="ma0 pa0 gh-members-list-name {{unless @member.name "gh-members-name-noname"}}">{{or @member.name @member.email}}</h3>
|
||||
{{#if @member.name}}
|
||||
<p class="ma0 pa0 middarkgrey f8 gh-members-list-email">{{@member.email}}</p>
|
||||
{{/if}}
|
||||
</div>
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data" data-test-table-data="details">
|
||||
<div class="flex items-center">
|
||||
<GhMemberAvatar @member={{@member}} @containerClass="w9 h9 mr3 flex-shrink-0" />
|
||||
<div class="w-80">
|
||||
<h3 class="ma0 pa0 gh-members-list-name {{unless @member.name "gh-members-name-noname"}}">{{or @member.name @member.email}}</h3>
|
||||
{{#if @member.name}}
|
||||
<p class="ma0 pa0 middarkgrey f8 gh-members-list-email">{{@member.email}}</p>
|
||||
{{/if}}
|
||||
</div>
|
||||
</LinkTo>
|
||||
{{#if this.hasMultipleTiers}}
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data" data-test-table-data="status">
|
||||
{{#if (not (is-empty @member.status))}}
|
||||
<span>{{capitalize @member.status}}</span>
|
||||
{{else}}
|
||||
<span class="midlightgrey">-</span>
|
||||
{{/if}}
|
||||
<div class="midgrey">{{this.tiers}}</div>
|
||||
</LinkTo>
|
||||
{{else}}
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data" data-test-table-data="status">
|
||||
{{#if (not (is-empty @member.status))}}
|
||||
<span>{{capitalize @member.status}}</span>
|
||||
{{else}}
|
||||
<span class="midlightgrey">-</span>
|
||||
{{/if}}
|
||||
</LinkTo>
|
||||
{{/if}}
|
||||
{{#if @newsletterEnabled}}
|
||||
{{#if (feature "emailAnalytics")}}
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data middarkgrey f8 {{unless @member.name "gh-members-list-open-rate-noname"}}" data-test-table-data="open-rate">
|
||||
{{#if (not (is-empty @member.emailOpenRate))}}
|
||||
<span>{{@member.emailOpenRate}}%</span>
|
||||
{{else}}
|
||||
<span class="midlightgrey">N/A</span>
|
||||
{{/if}}
|
||||
</LinkTo>
|
||||
{{/if}}
|
||||
{{/if}}
|
||||
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data middarkgrey f8 {{unless @member.name "gh-members-geolocation-noname"}}" data-test-table-data="location">
|
||||
{{#if (and @member.geolocation @member.geolocation.country)}}
|
||||
{{#if (and (eq @member.geolocation.country_code "US") @member.geolocation.region)}}
|
||||
{{@member.geolocation.region}}, US
|
||||
{{else}}
|
||||
{{#if @member.geolocation.country}}
|
||||
{{@member.geolocation.country}}
|
||||
{{else}}
|
||||
<span class="midlightgrey">Unknown</span>
|
||||
{{/if}}
|
||||
{{/if}}
|
||||
</div>
|
||||
</LinkTo>
|
||||
{{#if this.hasMultipleTiers}}
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data" data-test-table-data="status">
|
||||
{{#if (not (is-empty @member.status))}}
|
||||
<span>{{capitalize @member.status}}</span>
|
||||
{{else}}
|
||||
<span class="midlightgrey">Unknown</span>
|
||||
<span class="midlightgrey">-</span>
|
||||
{{/if}}
|
||||
<div class="midgrey">{{this.tiers}}</div>
|
||||
</LinkTo>
|
||||
{{else}}
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data" data-test-table-data="status">
|
||||
{{#if (not (is-empty @member.status))}}
|
||||
<span>{{capitalize @member.status}}</span>
|
||||
{{else}}
|
||||
<span class="midlightgrey">-</span>
|
||||
{{/if}}
|
||||
</LinkTo>
|
||||
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data middarkgrey f8" data-test-table-data="created-at">
|
||||
{{#if @member.createdAtUTC}}
|
||||
<div>{{moment-format (moment-site-tz @member.createdAtUTC) "DD MMM YYYY"}}</div>
|
||||
<div class="midlightgrey gh-members-list-subscribed-moment">{{moment-from-now @member.createdAtUTC}}</div>
|
||||
{{/if}}
|
||||
</LinkTo>
|
||||
|
||||
{{#each @filterColumns as |filterColumn|}}
|
||||
<GhMembersListItemColumn @member={{@member}} @filterColumn={{filterColumn}} />
|
||||
{{/each}}
|
||||
{{/if}}
|
||||
{{#if @newsletterEnabled}}
|
||||
{{#if (feature "emailAnalytics")}}
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data middarkgrey f8 {{unless @member.name "gh-members-list-open-rate-noname"}}" data-test-table-data="open-rate">
|
||||
{{#if (not (is-empty @member.emailOpenRate))}}
|
||||
<span>{{@member.emailOpenRate}}%</span>
|
||||
{{else}}
|
||||
<span class="midlightgrey">N/A</span>
|
||||
{{/if}}
|
||||
</LinkTo>
|
||||
{{/if}}
|
||||
{{/if}}
|
||||
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data middarkgrey f8 {{unless @member.name "gh-members-geolocation-noname"}}" data-test-table-data="location">
|
||||
{{#if (and @member.geolocation @member.geolocation.country)}}
|
||||
{{#if (and (eq @member.geolocation.country_code "US") @member.geolocation.region)}}
|
||||
{{@member.geolocation.region}}, US
|
||||
{{else}}
|
||||
{{#if @member.geolocation.country}}
|
||||
{{@member.geolocation.country}}
|
||||
{{else}}
|
||||
<span class="midlightgrey">Unknown</span>
|
||||
{{/if}}
|
||||
{{/if}}
|
||||
{{else}}
|
||||
<span class="midlightgrey">Unknown</span>
|
||||
{{/if}}
|
||||
</LinkTo>
|
||||
|
||||
<LinkTo @route="member" @model={{@member}} class="gh-list-data middarkgrey f8" data-test-table-data="created-at">
|
||||
{{#if @member.createdAtUTC}}
|
||||
<div>{{moment-format (moment-site-tz @member.createdAtUTC) "DD MMM YYYY"}}</div>
|
||||
<div class="midlightgrey gh-members-list-subscribed-moment">{{moment-from-now @member.createdAtUTC}}</div>
|
||||
{{/if}}
|
||||
</LinkTo>
|
||||
|
||||
{{#each @filterColumns as |filterColumn|}}
|
||||
<GhMembersListItemColumn @member={{@member}} @filterColumn={{filterColumn}} />
|
||||
{{/each}}
|
||||
</tr>
|
||||
|
|
|
|||
|
|
@ -1,5 +1,4 @@
|
|||
import Component from '@glimmer/component';
|
||||
import {get} from '@ember/object';
|
||||
import {inject as service} from '@ember/service';
|
||||
|
||||
export default class GhMembersListItem extends Component {
|
||||
|
|
@ -14,7 +13,7 @@ export default class GhMembersListItem extends Component {
|
|||
}
|
||||
|
||||
get tiers() {
|
||||
const tierData = get(this.args.member, 'tiers') || [];
|
||||
const tierData = this.args.member?.tiers || [];
|
||||
return tierData.map(tier => tier.name).join(', ');
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import Component from '@glimmer/component';
|
||||
import {action, get} from '@ember/object';
|
||||
import {action} from '@ember/object';
|
||||
import {tracked} from '@glimmer/tracking';
|
||||
|
||||
export default class MembersNewsletterPreference extends Component {
|
||||
|
|
@ -15,7 +15,7 @@ export default class MembersNewsletterPreference extends Component {
|
|||
return {
|
||||
name: d.name,
|
||||
description: d.description,
|
||||
subscribed: !!this.args.member?.get('newsletters')?.find((n) => {
|
||||
subscribed: !!this.args.member?.newsletters?.find((n) => {
|
||||
return n.id === d.id;
|
||||
}),
|
||||
id: d.id,
|
||||
|
|
@ -34,14 +34,12 @@ export default class MembersNewsletterPreference extends Component {
|
|||
return d.id === newsletter.id;
|
||||
});
|
||||
|
||||
// get() is required because member can be a proxy object when loaded
|
||||
// directly from the members list
|
||||
if (!event.target.checked) {
|
||||
updatedNewsletters = get(this.args.member, 'newsletters').filter((d) => {
|
||||
updatedNewsletters = this.args.member.newsletters.filter((d) => {
|
||||
return d.id !== newsletter.id;
|
||||
});
|
||||
} else {
|
||||
updatedNewsletters = get(this.args.member, 'newsletters').filter((d) => {
|
||||
updatedNewsletters = this.args.member.newsletters.filter((d) => {
|
||||
return d.id !== newsletter.id;
|
||||
}).concat(selectedNewsletter);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -68,9 +68,8 @@ export default class MemberController extends Controller {
|
|||
}
|
||||
|
||||
get subscribedAt() {
|
||||
// member can be a proxy object in a sparse array so .get is required
|
||||
let memberSince = moment(this.member.get('createdAtUTC')).from(moment());
|
||||
let createdDate = moment(this.member.get('createdAtUTC')).format('D MMM YYYY');
|
||||
let memberSince = moment(this.member.createdAtUTC).from(moment());
|
||||
let createdDate = moment(this.member.createdAtUTC).format('D MMM YYYY');
|
||||
return `${createdDate} (${memberSince})`;
|
||||
}
|
||||
|
||||
|
|
@ -133,7 +132,7 @@ export default class MemberController extends Controller {
|
|||
// if Cmd+S is pressed before the field loses focus make sure we're
|
||||
// saving the intended property values
|
||||
let scratchProps = scratchMember.getProperties(SCRATCH_PROPS);
|
||||
member.setProperties(scratchProps);
|
||||
Object.assign(member, scratchProps);
|
||||
|
||||
try {
|
||||
yield member.save();
|
||||
|
|
@ -178,7 +177,7 @@ export default class MemberController extends Controller {
|
|||
// Private -----------------------------------------------------------------
|
||||
|
||||
_saveMemberProperty(propKey, newValue) {
|
||||
let currentValue = this.member.get(propKey);
|
||||
let currentValue = this.member[propKey];
|
||||
|
||||
if (newValue && typeof newValue === 'string') {
|
||||
newValue = newValue.trim();
|
||||
|
|
@ -189,6 +188,6 @@ export default class MemberController extends Controller {
|
|||
return;
|
||||
}
|
||||
|
||||
this.member.set(propKey, newValue);
|
||||
this.member[propKey] = newValue;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -30,9 +30,7 @@ export default class MembersRoute extends AdminRoute {
|
|||
setupController(controller, member) {
|
||||
super.setupController(...arguments);
|
||||
if (this._requiresBackgroundRefresh) {
|
||||
// `member` is passed directly in `<LinkTo>` so it can be a proxy
|
||||
// object used by the sparse list requiring the use of .get()
|
||||
controller.fetchMemberTask.perform(member.get('id'));
|
||||
controller.fetchMemberTask.perform(member.id);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -23,9 +23,7 @@ export default class OffersRoute extends AdminRoute {
|
|||
super.setupController(...arguments);
|
||||
|
||||
if (this._requiresBackgroundRefresh) {
|
||||
// `offer` is passed directly in `<LinkTo>` so it can be a proxy
|
||||
// object used by the sparse list requiring the use of .get()
|
||||
controller.fetchOfferTask.perform(offer.get('id'));
|
||||
controller.fetchOfferTask.perform(offer.id);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -126,12 +126,19 @@
|
|||
</tr>
|
||||
</thead>
|
||||
<VerticalCollection @tagName="tbody" @items={{this.members}} @key="id" @containerSelector=".gh-list-scrolling" @estimateHeight={{69}} @staticHeight={{true}} @bufferSize={{20}} as |member|>
|
||||
<GhMembersListItem
|
||||
@newsletterEnabled={{not-eq this.settings.editorDefaultEmailRecipients "disabled"}}
|
||||
@member={{member}}
|
||||
@filterColumns={{this.filterColumns}}
|
||||
data-test-member={{member.id}}
|
||||
/>
|
||||
{{#if member.is_loading}}
|
||||
<GhMembersListItemLoading
|
||||
@newsletterEnabled={{not-eq this.settings.editorDefaultEmailRecipients "disabled"}}
|
||||
@filterColumns={{this.filterColumns}}
|
||||
/>
|
||||
{{else}}
|
||||
<GhMembersListItem
|
||||
@newsletterEnabled={{not-eq this.settings.editorDefaultEmailRecipients "disabled"}}
|
||||
@member={{member.content}}
|
||||
@filterColumns={{this.filterColumns}}
|
||||
data-test-member={{member.id}}
|
||||
/>
|
||||
{{/if}}
|
||||
</VerticalCollection>
|
||||
</table>
|
||||
</div>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue