FIX: Prevent 'NaN' display by hiding visitor stats on /about until they're ready (#29334)

The visitor stats on the /about page were previously showing as `NaN` immediately after enabling the `display_eu_visitor_stats` site setting because the stats for the /about page are cached and updated once every 30 minutes in a sidekiq job. The `NaN` would go away upon the next run of the relevant sidekiq job, but it's not good UX to display a cryptic `NaN` until the job runs. So, this commit ensures that the visitor stats is not displayed at all until the visitor stats is calculated and available.

Internal topic: t/128480.
This commit is contained in:
Osama Sayegh 2024-10-22 19:29:44 +03:00 committed by GitHub
parent 856182c7c6
commit 91c674f0bc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 63 additions and 5 deletions

View File

@ -120,7 +120,7 @@ export default class AboutPage extends Component {
}, },
]; ];
if (this.siteSettings.display_eu_visitor_stats) { if (this.displayVisitorStats) {
list.splice(2, 0, { list.splice(2, 0, {
icon: "user-secret", icon: "user-secret",
class: "visitors", class: "visitors",
@ -137,6 +137,14 @@ export default class AboutPage extends Component {
return list.concat(this.siteActivitiesFromPlugins()); return list.concat(this.siteActivitiesFromPlugins());
} }
get displayVisitorStats() {
return (
this.siteSettings.display_eu_visitor_stats &&
typeof this.args.model.stats.eu_visitors_7_days === "number" &&
typeof this.args.model.stats.visitors_7_days === "number"
);
}
get contactInfo() { get contactInfo() {
const url = escape(this.args.model.contact_url || ""); const url = escape(this.args.model.contact_url || "");
const email = escape(this.args.model.contact_email || ""); const email = escape(this.args.model.contact_email || "");
@ -292,7 +300,7 @@ export default class AboutPage extends Component {
</div> </div>
{{/each}} {{/each}}
</div> </div>
{{#if this.siteSettings.display_eu_visitor_stats}} {{#if this.displayVisitorStats}}
<p class="about traffic-info-footer"><small <p class="about traffic-info-footer"><small
>{{this.trafficInfoFooter}}</small></p> >{{this.trafficInfoFooter}}</small></p>
{{/if}} {{/if}}

View File

@ -54,4 +54,17 @@ export default class AboutController extends Controller {
} }
return Array.from(set); return Array.from(set);
} }
@discourseComputed(
"model.stats.visitors_7_days",
"model.stats.eu_visitors_7_days",
"siteSettings.display_eu_visitor_stats"
)
displayVisitorStats(visitors, euVisitors, displayEuVisitorStats) {
return (
displayEuVisitorStats &&
typeof euVisitors === "number" &&
typeof visitors === "number"
);
}
} }

View File

@ -139,7 +139,7 @@
<td>{{number this.model.stats.active_users_30_days}}</td> <td>{{number this.model.stats.active_users_30_days}}</td>
<td>&mdash;</td> <td>&mdash;</td>
</tr> </tr>
{{#if this.siteSettings.display_eu_visitor_stats}} {{#if this.displayVisitorStats}}
<tr class="about-visitor-count"> <tr class="about-visitor-count">
<td>{{i18n "about.visitor_count"}}</td> <td>{{i18n "about.visitor_count"}}</td>
<td>{{number this.model.stats.visitors_last_day}}</td> <td>{{number this.model.stats.visitors_last_day}}</td>
@ -183,7 +183,7 @@
{{/each}} {{/each}}
</tbody> </tbody>
</table> </table>
{{#if this.siteSettings.display_eu_visitor_stats}} {{#if this.displayVisitorStats}}
<p class="about stats-table-footer"><small <p class="about stats-table-footer"><small
>{{this.statsTableFooter}}</small></p> >{{this.statsTableFooter}}</small></p>
{{/if}} {{/if}}

View File

@ -3,6 +3,7 @@ import { module, test } from "qunit";
import AboutPage from "discourse/components/about-page"; import AboutPage from "discourse/components/about-page";
import { withPluginApi } from "discourse/lib/plugin-api"; import { withPluginApi } from "discourse/lib/plugin-api";
import { setupRenderingTest } from "discourse/tests/helpers/component-test"; import { setupRenderingTest } from "discourse/tests/helpers/component-test";
import I18n from "discourse-i18n";
function createModelObject({ function createModelObject({
title = "My Forums", title = "My Forums",
@ -61,4 +62,40 @@ module("Integration | Component | about-page", function (hooks) {
) )
.hasText("in the last 3 weeks"); .hasText("in the last 3 weeks");
}); });
test("visitor stats are not rendered if they're not available in the model", async function (assert) {
this.siteSettings.display_eu_visitor_stats = true;
let model = createModelObject({
stats: {},
});
await render(<template><AboutPage @model={{model}} /></template>);
assert
.dom(".about__activities-item.visitors")
.doesNotExist("visitors stats item is not rendered");
model = createModelObject({
stats: {
eu_visitors_7_days: 13,
eu_visitors_30_days: 30,
visitors_7_days: 33,
visitors_30_days: 103,
},
});
await render(<template><AboutPage @model={{model}} /></template>);
assert
.dom(".about__activities-item.visitors")
.exists("visitors stats item is rendered");
assert
.dom(".about__activities-item.visitors .about__activities-item-count")
.hasText(
I18n.messageFormat("about.activities.visitors_MF", {
total_count: 33,
eu_count: 13,
total_formatted_number: "33",
eu_formatted_number: "13",
})
);
});
}); });

View File

@ -2263,7 +2263,7 @@ en:
tos_url: "If you have a Terms of Service document hosted elsewhere that you want to use, provide the full URL here." tos_url: "If you have a Terms of Service document hosted elsewhere that you want to use, provide the full URL here."
privacy_policy_url: "If you have a Privacy Policy document hosted elsewhere that you want to use, provide the full URL here." privacy_policy_url: "If you have a Privacy Policy document hosted elsewhere that you want to use, provide the full URL here."
log_anonymizer_details: "Whether to keep a user's details in the log after being anonymized." log_anonymizer_details: "Whether to keep a user's details in the log after being anonymized."
display_eu_visitor_stats: "Show number of global and EU visitors on the /about page." display_eu_visitor_stats: "Show number of global and EU visitors on the /about page. It may take a few minutes for the stats to appear after turning on this setting."
newuser_spam_host_threshold: "How many times a new user can post a link to the same host within their `newuser_spam_host_threshold` posts before being considered spam." newuser_spam_host_threshold: "How many times a new user can post a link to the same host within their `newuser_spam_host_threshold` posts before being considered spam."