From 17a6217cc76188e39130621996746d9fa5b73b7c Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Thu, 4 May 2023 11:11:08 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20members=20breadcrumbs=20?= =?UTF-8?q?when=20not=20coming=20from=20analytics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes https://github.com/TryGhost/Team/issues/2404 This change introduces a new 'post' query parameter to the members and member routes. Previously, the members route would check if the previous route was the analytics page, and then show the breadcrumbs to go back to the analytics page. But when navigating to the members page from the menu, we don't want to show the breadcrumbs. To accomplish this, the routes that point to the members page from the analytics page now specifically pass on the post id in the query parameters. The query parameter is then passed on from the members page to the member page. `directlyFromAnalytics` is still used in the member route, to know wheter we came from the members page or from the analytics page (changes the breadcrumbs). This doesn't need to go via a query parameter (figured that would make the url too long/complex). The resetController method is now implemented and resets the filter and/or fromAnalytics post id if required (when going from members to member, we don't want to reset it because the we would lose the filter going back). --- .../components/members/list-item-column.hbs | 2 +- .../app/components/members/list-item.hbs | 14 +++---- .../components/posts/post-activity-feed.hbs | 4 +- .../posts/post-activity-feed/footer-links.hbs | 8 ++-- ghost/admin/app/controllers/member.js | 15 ++++++- ghost/admin/app/controllers/members.js | 12 +++++- ghost/admin/app/routes/member.js | 39 +++++++----------- ghost/admin/app/routes/members.js | 40 +++++++------------ ghost/admin/app/templates/member.hbs | 2 +- ghost/admin/app/templates/members.hbs | 1 + 10 files changed, 67 insertions(+), 70 deletions(-) diff --git a/ghost/admin/app/components/members/list-item-column.hbs b/ghost/admin/app/components/members/list-item-column.hbs index ed268794fb..7cd60d8666 100644 --- a/ghost/admin/app/components/members/list-item-column.hbs +++ b/ghost/admin/app/components/members/list-item-column.hbs @@ -1,4 +1,4 @@ - + {{#if this.columnValue}}
{{#if this.columnValue.icon}} diff --git a/ghost/admin/app/components/members/list-item.hbs b/ghost/admin/app/components/members/list-item.hbs index e5aa48071c..ea0f892eaa 100644 --- a/ghost/admin/app/components/members/list-item.hbs +++ b/ghost/admin/app/components/members/list-item.hbs @@ -1,5 +1,5 @@ - +
@@ -11,7 +11,7 @@
{{#if this.hasMultipleTiers}} - + {{#if (not (is-empty @member.status))}} {{capitalize @member.status}} {{else}} @@ -20,7 +20,7 @@
{{this.tiers}}
 {{else}} - + {{#if (not (is-empty @member.status))}} {{capitalize @member.status}} {{else}} @@ -29,7 +29,7 @@ {{/if}} {{#if @newsletterEnabled}} - + {{#if (not (is-empty @member.emailOpenRate))}} {{@member.emailOpenRate}}% {{else}} @@ -38,7 +38,7 @@ {{/if}} - + {{#if (and @member.geolocation @member.geolocation.country)}} {{#if (and (eq @member.geolocation.country_code "US") @member.geolocation.region)}} {{@member.geolocation.region}}, US @@ -54,7 +54,7 @@ {{/if}} - + {{#if @member.createdAtUTC}}
{{moment-format (moment-site-tz @member.createdAtUTC) "DD MMM YYYY"}}
{{moment-from-now @member.createdAtUTC}}
@@ -62,6 +62,6 @@ {{#each @filterColumns as |filterColumn|}} - + {{/each}} diff --git a/ghost/admin/app/components/posts/post-activity-feed.hbs b/ghost/admin/app/components/posts/post-activity-feed.hbs index a66fe1f2f6..c227be9558 100644 --- a/ghost/admin/app/components/posts/post-activity-feed.hbs +++ b/ghost/admin/app/components/posts/post-activity-feed.hbs @@ -46,7 +46,7 @@
- {{parsedEvent.subject}} + {{parsedEvent.subject}}
{{svg-jar parsedEvent.icon }} @@ -95,7 +95,7 @@ {{svg-jar "filter"}} View members diff --git a/ghost/admin/app/components/posts/post-activity-feed/footer-links.hbs b/ghost/admin/app/components/posts/post-activity-feed/footer-links.hbs index f6a0e0780e..396bf0e9c5 100644 --- a/ghost/admin/app/components/posts/post-activity-feed/footer-links.hbs +++ b/ghost/admin/app/components/posts/post-activity-feed/footer-links.hbs @@ -3,7 +3,7 @@ Sent @@ -13,7 +13,7 @@ Opened @@ -23,7 +23,7 @@ Clicked @@ -34,7 +34,7 @@ {{link.label}} diff --git a/ghost/admin/app/controllers/member.js b/ghost/admin/app/controllers/member.js index 2efb0b3fca..f64cf52edb 100644 --- a/ghost/admin/app/controllers/member.js +++ b/ghost/admin/app/controllers/member.js @@ -19,6 +19,10 @@ export default class MemberController extends Controller { @service router; @service store; + queryParams = [ + {postAnalytics: 'post'} + ]; + @tracked isLoading = false; @tracked showImpersonateMemberModal = false; @tracked modalLabel = null; @@ -27,8 +31,15 @@ export default class MemberController extends Controller { _previousLabels = null; _previousNewsletters = null; - directlyFromAnalytics = false; - fromAnalytics = null; + @tracked directlyFromAnalytics = false; + @tracked postAnalytics = null; + + get fromAnalytics() { + if (!this.postAnalytics) { + return null; + } + return [this.postAnalytics]; + } constructor() { super(...arguments); diff --git a/ghost/admin/app/controllers/members.js b/ghost/admin/app/controllers/members.js index 01345210ee..3b54f9828e 100644 --- a/ghost/admin/app/controllers/members.js +++ b/ghost/admin/app/controllers/members.js @@ -44,7 +44,8 @@ export default class MembersController extends Controller { {paidParam: 'paid'}, {searchParam: 'search'}, {orderParam: 'order'}, - {filterParam: 'filter'} + {filterParam: 'filter'}, + {postAnalytics: 'post'} ]; @tracked members = A([]); @@ -67,7 +68,14 @@ export default class MembersController extends Controller { /** * Flag used to determine if we should return to the analytics page */ - fromAnalytics = null; + @tracked postAnalytics = null; + + get fromAnalytics() { + if (!this.postAnalytics) { + return null; + } + return [this.postAnalytics]; + } paidParams = PAID_PARAMS; diff --git a/ghost/admin/app/routes/member.js b/ghost/admin/app/routes/member.js index 3874a67867..b0b245376b 100644 --- a/ghost/admin/app/routes/member.js +++ b/ghost/admin/app/routes/member.js @@ -8,8 +8,11 @@ export default class MembersRoute extends AdminRoute { @service modals; @service router; + queryParams = { + postAnalytics: {refreshModel: false} + }; + _requiresBackgroundRefresh = true; - fromAnalytics = null; constructor() { super(...arguments); @@ -39,25 +42,17 @@ export default class MembersRoute extends AdminRoute { controller.directlyFromAnalytics = false; if (transition.from?.name === 'posts.analytics') { - // Sadly transition.from.params is not reliable to use (not populated on transitions) - const oldParams = transition.router?.oldState?.params['posts.analytics'] ?? {}; - - // We need to store analytics in 'this' to have it accessible for the member route - this.fromAnalytics = Object.values(oldParams); - controller.fromAnalytics = this.fromAnalytics; controller.directlyFromAnalytics = true; - } else if (transition.from?.metadata?.fromAnalytics) { - // Handle returning from member route - const fromAnalytics = transition.from?.metadata.fromAnalytics ?? null; - controller.fromAnalytics = fromAnalytics; - this.fromAnalytics = fromAnalytics; - } else if (transition.from?.name === 'members.index' && transition.from?.parent?.name === 'members') { - const fromAnalytics = transition.from?.parent?.metadata.fromAnalytics ?? null; - controller.fromAnalytics = fromAnalytics; - this.fromAnalytics = fromAnalytics; - } else { - controller.fromAnalytics = null; - this.fromAnalytics = null; + } + } + + resetController(controller, isExiting) { + super.resetController(...arguments); + + // Make sure we clear + if (isExiting && controller.postAnalytics) { + controller.set('postAnalytics', null); + controller.set('directlyFromAnalytics', false); } } @@ -123,10 +118,4 @@ export default class MembersRoute extends AdminRoute { titleToken() { return this.controller.member.name; } - - buildRouteInfoMetadata() { - return { - fromAnalytics: this.fromAnalytics - }; - } } diff --git a/ghost/admin/app/routes/members.js b/ghost/admin/app/routes/members.js index 2a0be38a0e..67207e2620 100644 --- a/ghost/admin/app/routes/members.js +++ b/ghost/admin/app/routes/members.js @@ -5,14 +5,13 @@ export default class MembersRoute extends AdminRoute { @service store; @service feature; - fromAnalytics = false; - queryParams = { label: {refreshModel: true}, searchParam: {refreshModel: true, replace: true}, paidParam: {refreshModel: true}, orderParam: {refreshModel: true}, - filterParam: {refreshModel: true} + filterParam: {refreshModel: true}, + postAnalytics: {refreshModel: false} }; beforeModel() { @@ -26,39 +25,28 @@ export default class MembersRoute extends AdminRoute { } // trigger a background load of members plus labels for filter dropdown - setupController(controller, model, transition) { + setupController(controller) { super.setupController(...arguments); controller.fetchLabelsTask.perform(); - - if (transition.from?.name === 'posts.analytics') { - // Sadly transition.from.params is not reliable to use (not populated on transitions) - const oldParams = transition.router?.oldState?.params['posts.analytics'] ?? {}; - - // We need to store analytics in 'this' to have it accessible for the member route - this.fromAnalytics = Object.values(oldParams); - controller.fromAnalytics = this.fromAnalytics; - } else if (transition.from?.metadata?.fromAnalytics) { - // Handle returning from member route - const fromAnalytics = transition.from?.metadata.fromAnalytics ?? null; - controller.fromAnalytics = fromAnalytics; - this.fromAnalytics = fromAnalytics; - } else { - controller.fromAnalytics = null; - this.fromAnalytics = null; - } } - resetController() { + resetController(controller, _isExiting, transition) { super.resetController(...arguments); - // don't reset fromAnalytics here, we need to reuse it. We reset it in setup - //controller.fromAnalytics = null; + + if (controller.postAnalytics) { + controller.set('postAnalytics', null); + // Only reset filters if we are not going to member route + // Otherwise the filters will be gone if we return + if (!transition?.to?.name?.startsWith('member')) { + controller.set('filterParam', null); + } + } } buildRouteInfoMetadata() { return { titleToken: 'Members', - mainClasses: ['gh-main-fullwidth'], - fromAnalytics: this.fromAnalytics + mainClasses: ['gh-main-fullwidth'] }; } } diff --git a/ghost/admin/app/templates/member.hbs b/ghost/admin/app/templates/member.hbs index b231d41e02..a5b7186ee1 100644 --- a/ghost/admin/app/templates/member.hbs +++ b/ghost/admin/app/templates/member.hbs @@ -13,7 +13,7 @@ {{#unless this.directlyFromAnalytics}} {{svg-jar "arrow-right-small"}} - + Members {{/unless}} diff --git a/ghost/admin/app/templates/members.hbs b/ghost/admin/app/templates/members.hbs index 4b5fac4e01..9bd9c98cdc 100644 --- a/ghost/admin/app/templates/members.hbs +++ b/ghost/admin/app/templates/members.hbs @@ -154,6 +154,7 @@ @newsletterEnabled={{and (not-eq this.settings.editorDefaultEmailRecipients "disabled") this.settings.emailTrackOpens}} @member={{member.content}} @filterColumns={{this.filterColumns}} + @query={{hash postAnalytics=this.postAnalytics}} data-test-member={{member.id}} /> {{/if}}