Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: added

Locations: offer Regions in the widget's "View by" control.
Comment thread
kangzj marked this conversation as resolved.
Outdated
Original file line number Diff line number Diff line change
Expand Up @@ -1100,9 +1100,16 @@ function getLocationRows(
}

if ( geoMode === 'region' ) {
const countryCode = query.get( 'filter_by_country' ) || 'US';
const countryCode = query.get( 'filter_by_country' );
const regionRows = isComparison ? REGION_COMPARISON_ROWS_BY_COUNTRY : REGION_ROWS_BY_COUNTRY;

// Unfiltered, the endpoint returns the top regions worldwide.
if ( ! countryCode ) {
return Object.values( regionRows )
.flat()
.sort( ( a, b ) => b.views - a.views );
}

return regionRows[ countryCode ] ?? regionRows.US;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { AnchorHTMLAttributes, ReactNode } from 'react';
/**
* Internal dependencies
Expand Down Expand Up @@ -38,21 +39,40 @@ jest.mock( '@wordpress/route', () => ( {
useSearch: () => ( {} ),
} ) );

// Google Charts loads asynchronously and is outside this widget's concern.
jest.mock( '@jetpack-premium-analytics/widgets-toolkit', () => ( {
...jest.requireActual( '@jetpack-premium-analytics/widgets-toolkit' ),
GeoChart: () => <div data-testid="geo-chart" />,
} ) );

// Typed off the hook so the mocked state and the rows passed to
// `mockReturnValue` are type-checked rather than cast away.
type LocationViewsState = ReturnType< typeof import('../use-location-views').default >;

const LOADING_STATE: LocationViewsState = {
data: [],
hasComparison: false,
isLoading: true,
isFetching: true,
hasData: false,
isError: false,
isPlaceholderData: false,
refetch: () => {},
};

const mockUseLocationViews = jest.fn( () => LOADING_STATE );

jest.mock( '../use-location-views', () => ( {
__esModule: true,
default: () => ( {
data: [],
comparisonData: [],
hasComparison: false,
isLoading: true,
isFetching: true,
hasData: false,
isError: false,
isPlaceholderData: false,
} ),
default: ( ...args: unknown[] ) => mockUseLocationViews( ...( args as [] ) ),
} ) );

describe( 'LocationsWidget', () => {
beforeEach( () => {
mockUseLocationViews.mockReset();
mockUseLocationViews.mockReturnValue( LOADING_STATE );
} );

it( 'links to the Locations report', () => {
render( <LocationsWidget attributes={ {} } /> );

Expand All @@ -61,4 +81,71 @@ describe( 'LocationsWidget', () => {
expect.stringContaining( '/reports/locations' )
);
} );

it.each( [
[ undefined, 'countries' ],
Comment thread
kangzj marked this conversation as resolved.
Outdated
[ 'country', 'countries' ],
[ 'region', 'regions' ],
[ 'city', 'cities' ],
] as const )( 'opens the %s granularity on the %s report tab', ( geoGranularity, section ) => {
render( <LocationsWidget attributes={ geoGranularity ? { geoGranularity } : {} } /> );

expect( screen.getByRole( 'link', { name: 'View all' } ) ).toHaveAttribute(
'href',
expect.stringContaining( `section=${ section }` )
);
} );

// Regions mode is worldwide; only the country drill-down scopes it.
it( 'requests unfiltered region rows in Regions mode', () => {
render( <LocationsWidget attributes={ { geoGranularity: 'region' } } /> );

expect( mockUseLocationViews ).toHaveBeenLastCalledWith(
expect.objectContaining( { geoMode: 'region', countryFilter: undefined } )
);
} );

// Without the reset, the drill-down survives the trip through Regions and
// Countries mode comes back scoped to one country instead of listing them all.
it( 'drops a drilled-down country when switching to Regions', async () => {
mockUseLocationViews.mockReturnValue( {
...LOADING_STATE,
data: [
{
key: 'US:United States',
label: 'United States',
countryCode: 'US',
countryFull: 'United States',
value: 10,
region: '',
},
],
isLoading: false,
isFetching: false,
hasData: true,
} );

const { rerender } = render( <LocationsWidget attributes={ { geoGranularity: 'country' } } /> );
await userEvent.click(
screen.getByRole( 'button', { name: 'View regions in United States' } )
);

expect( mockUseLocationViews ).toHaveBeenLastCalledWith(
expect.objectContaining( { geoMode: 'region', countryFilter: 'US' } )
);

rerender( <LocationsWidget attributes={ { geoGranularity: 'region' } } /> );

expect( mockUseLocationViews ).toHaveBeenLastCalledWith(
expect.objectContaining( { geoMode: 'region', countryFilter: undefined } )
);
// Regions mode keeps the map alongside the leaderboard.
expect( screen.getByTestId( 'geo-chart' ) ).toBeInTheDocument();

rerender( <LocationsWidget attributes={ { geoGranularity: 'country' } } /> );

expect( mockUseLocationViews ).toHaveBeenLastCalledWith(
expect.objectContaining( { geoMode: 'country', countryFilter: undefined } )
);
} );
} );
61 changes: 38 additions & 23 deletions projects/packages/premium-analytics/widgets/locations/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ const MISSING_MAP_ERROR_MESSAGE = 'Requested map does not exist';
// load each country pays the failed draw (a brief error flash) at most once.
const runtimeUnsupportedProvinceMapCountries = new Set< string >();

type GeoGranularity = NonNullable< LocationsAttributes[ 'geoGranularity' ] >;
// Tab ids owned by the Locations report; `ReportLink` takes a bare string, so
// naming them here is what catches a typo at build time.
type LocationsReportSection = 'countries' | 'regions' | 'cities';

const REPORT_SECTIONS: Record< GeoGranularity, LocationsReportSection > = {
country: 'countries',
region: 'regions',
city: 'cities',
};

function getGeoChartCountryId( countryCode: string ): string {
if ( countryCode.toUpperCase() === 'TW' ) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hum why would we need this special handling here 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looked at it. I think this line does nothing now. It came in with #50251 the "Avoid unsupported Taiwan map" commit.

The actual fix turned out to be runtimeUnsupportedProvinceMapCountries. Worth deleting in a follow-up rather than here, keep the .toUpperCase() though.

cc @dognose24 for confirming.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirmed — that line came out of the TW "Requested map does not exist" fight, and you're right that the actual fix was runtimeUnsupportedProvinceMapCountries; the province-map error happens on the region: 'TW' + resolution: 'provinces' path, which this row-value mapping never touches. One nuance before deleting: it does change the world-map datatable value from 'TW' to 'Taiwan'. GeoChart's regions mode documents ISO alpha-2 as accepted, so the bare code should highlight the same — worth a quick check against the Cities-mode story (its mock has TW rows) when the follow-up removes it. Keeping .toUpperCase() 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Leaving the TWTaiwan mapping in place here as agreed, .toUpperCase() included. Happy to open the follow-up to remove it, with the Cities-mode story check against the TW rows as the verification step.

return 'Taiwan';
Expand Down Expand Up @@ -91,9 +102,10 @@ function LocationsInner( { max, geoGranularity }: LocationsInnerProps ) {
} = useWidgetDrillDown< DrillDownCountry >();

// The "View by" control lives in the widget host header (the
// `relevance: 'high'` attribute). City mode disables country drill-down.
// `relevance: 'high'` attribute). Only Countries mode drills down, so leaving
// the other modes would strand a selected country the user can't clear.
Comment thread
kangzj marked this conversation as resolved.
useEffect( () => {
if ( geoGranularity === 'city' ) {
if ( geoGranularity !== 'country' ) {
clearSelectedCountry();
}
}, [ clearSelectedCountry, geoGranularity ] );
Expand Down Expand Up @@ -134,11 +146,15 @@ function LocationsInner( { max, geoGranularity }: LocationsInnerProps ) {
const useCountryFallbackMap =
renderGeoMode === 'region' && !! renderSelectedCountry && ! useProvinceMap;
const fallbackCountry = useCountryFallbackMap ? renderSelectedCountry : undefined;
const useCityCountryMap = renderGeoMode === 'city';
const cityCountryRows = useMemo( () => {
// Cities, and Regions outside a country drill-down, span the whole world.
// Google GeoChart can't place either row type on the world map, so both are
// summed back up to their country.
const useCountrySummaryMap =
renderGeoMode === 'city' || ( renderGeoMode === 'region' && ! renderSelectedCountry );
const countrySummaryRows = useMemo( () => {
const countryRows = new Map< string, { countryFull: string; value: number } >();

if ( ! useCityCountryMap ) {
if ( ! useCountrySummaryMap ) {
return [];
}

Expand All @@ -152,7 +168,7 @@ function LocationsInner( { max, geoGranularity }: LocationsInnerProps ) {
} );

return Array.from( countryRows.entries() );
}, [ data, useCityCountryMap ] );
}, [ data, useCountrySummaryMap ] );
const handleGeoChartError = useCallback(
( error: GeoChartError ) => {
const message = `${ error.message ?? '' } ${ error.detailedMessage ?? '' }`;
Expand Down Expand Up @@ -201,9 +217,10 @@ function LocationsInner( { max, geoGranularity }: LocationsInnerProps ) {
);

const geoData = useMemo( (): GeoData => {
const useLocationHeader = renderGeoMode === 'region' && ! useCountryFallbackMap;
// Only the provinces map plots sub-country rows; every other map is
// country-scoped, whatever the leaderboard beside it lists.
const header: GoogleDataTableColumn[] = [
useLocationHeader
useProvinceMap
? __( 'Location', 'jetpack-premium-analytics-pkg' )
: __( 'Country', 'jetpack-premium-analytics-pkg' ),
__( 'Views', 'jetpack-premium-analytics-pkg' ),
Expand All @@ -227,10 +244,10 @@ function LocationsInner( { max, geoGranularity }: LocationsInnerProps ) {
];
}

if ( useCityCountryMap ) {
if ( useCountrySummaryMap ) {
return [
header,
...cityCountryRows.map(
...countrySummaryRows.map(
( [ countryCode, location ] ): GoogleDataTableRow => [
{
v: getGeoChartCountryId( countryCode ),
Expand All @@ -244,14 +261,7 @@ function LocationsInner( { max, geoGranularity }: LocationsInnerProps ) {

const rows: GoogleDataTableRow[] = data.map( location => [ location.label, location.value ] );
return [ header, ...rows ];
}, [
cityCountryRows,
data,
fallbackCountry,
renderGeoMode,
useCityCountryMap,
useCountryFallbackMap,
] );
}, [ countrySummaryRows, data, fallbackCountry, useCountrySummaryMap, useProvinceMap ] );

const leaderboardData = useMemo( () => {
const maxValue = getCombinedPeriodMax(
Expand Down Expand Up @@ -382,17 +392,22 @@ function LocationsInner( { max, geoGranularity }: LocationsInnerProps ) {
*/
export default function Locations( { attributes = {} }: LocationsWidgetProps ) {
const max = attributes?.max ?? 10;
const geoGranularity = attributes?.geoGranularity ?? 'country';
// Attributes are persisted, so a stale layout can carry a granularity this
// widget no longer knows. Normalize once, before it becomes both the endpoint
// path segment and the report tab.
const storedGranularity = attributes?.geoGranularity ?? 'country';
// `in` would also accept inherited keys such as `toString`, which would then
// reach the endpoint as a path segment.
const geoGranularity = Object.prototype.hasOwnProperty.call( REPORT_SECTIONS, storedGranularity )
? storedGranularity
: 'country';
Comment thread
kangzj marked this conversation as resolved.
Outdated

return (
<WidgetRoot attributes={ attributes }>
<div className={ styles.root }>
<LocationsInner max={ max } geoGranularity={ geoGranularity } />
<WidgetFooter>
<ReportLink
report="locations"
section={ geoGranularity === 'city' ? 'cities' : 'countries' }
/>
<ReportLink report="locations" section={ REPORT_SECTIONS[ geoGranularity ] } />
</WidgetFooter>
</div>
</WidgetRoot>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,15 @@ const meta = {
},
geoGranularity: {
control: 'radio',
options: [ 'country', 'city' ],
options: [ 'country', 'region', 'city' ],
description: 'The "View by" toolbar attribute rendered by the widget host.',
},
},
parameters: {
docs: {
description: {
component:
'The "Locations" widget. Shows visitor views by country or city, with country drill-down into regions, using the global dashboard date range. The Countries/Cities view is the `geoGranularity` attribute (`relevance: \'high\'`), exposed as a control by the widget host.',
'The "Locations" widget. Shows visitor views by country, region, or city, with country drill-down into regions, using the global dashboard date range. The Countries/Regions/Cities view is the `geoGranularity` attribute (`relevance: \'high\'`), exposed as a control by the widget host.',
},
},
},
Expand All @@ -123,6 +123,14 @@ export const WithComparison: StoryObj< LocationsStoryControls > = {
decorators: [ withWidgetCanvas, withStoryRouter ],
};

// Regions mode — region rows worldwide in the leaderboard, aggregated by country
// on the map.
export const RegionsMode: StoryObj< LocationsStoryControls > = {
render: renderLocationsWidget,
args: { withComparison: false, geoGranularity: 'region' },
decorators: [ withWidgetCanvas, withStoryRouter ],
};

// Cities mode — city rows in the leaderboard, aggregated by country on the map.
export const CitiesMode: StoryObj< LocationsStoryControls > = {
render: renderLocationsWidget,
Expand Down Expand Up @@ -190,7 +198,7 @@ export const WidgetDashboardWithWidget: DashboardStory = {
},
geoGranularity: {
control: 'radio',
options: [ 'country', 'city' ],
options: [ 'country', 'region', 'city' ],
description: 'The "View by" toolbar attribute rendered by the widget host.',
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ import { SelectField } from '@jetpack-premium-analytics/fields';

export type LocationsAttributes = {
max?: number;
geoGranularity?: 'country' | 'city';
geoGranularity?: 'country' | 'region' | 'city';
};

/**
* Widget type definition.
*
* Ported from the Jetpack Stats "Locations" module. v1 ships Countries mode
* (with region drill-down) and Cities mode via the `location-views/{geoMode}`
* endpoint. City rows are listed in the leaderboard and summarized on the map
* by country.
* Ported from the Jetpack Stats "Locations" module. Countries, Regions, and
* Cities modes all read the `location-views/{geoMode}` endpoint; Countries mode
* additionally drills down into one country's regions. Region and city rows are
* listed in the leaderboard and summarized on the map by country.
*
* Data: fetched via the PA proxy at `stats/location-views/{country|region|city}`.
* Date range comes from WidgetRoot's reportParams (the shared dashboard date
Expand Down Expand Up @@ -49,6 +49,10 @@ export default {
label: __( 'Countries', 'jetpack-premium-analytics-pkg' ),
value: 'country',
},
{
label: __( 'Regions', 'jetpack-premium-analytics-pkg' ),
value: 'region',
},
{
label: __( 'Cities', 'jetpack-premium-analytics-pkg' ),
value: 'city',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: added

Top locations: add Regions to the "View by" control, listing the top regions worldwide.
Loading