Merge pull request #5964 from facebook/fixup/chartjs-warnings

Fix chartjs warnings

frontend/src/components/projectDetail/index.js

@@ -370,6 +370,29 @@ export const ProjectDetail = (props) => {
   );
 };
 
+const GeometryPropType = PropTypes.shape({
+  type: PropTypes.oneOf([
+    'Point',
+    'MultiPoint',
+    'LineString',
+    'MultiLineString',
+    'Polygon',
+    'MultiPolygon',
+    'GeometryCollection',
+  ]),
+  coordinates: PropTypes.array,
+  geometries: PropTypes.array,
+});
+const FeaturePropType = PropTypes.shape({
+  type: PropTypes.oneOf(['Feature']),
+  geometry: GeometryPropType,
+  properties: PropTypes.object,
+});
+const FeatureCollectionPropType = PropTypes.shape({
+  type: PropTypes.oneOf(['FeatureCollection']),
+  features: PropTypes.arrayOf(FeaturePropType).isRequired,
+});
+
 ProjectDetail.propTypes = {
   project: PropTypes.shape({
     projectId: PropTypes.number,
@@ -393,10 +416,11 @@ ProjectDetailMap.propTypes = {
     areaOfInterest: PropTypes.object,
     priorityAreas: PropTypes.arrayOf(PropTypes.object),
   }).isRequired,
-  tasks: PropTypes.arrayOf(PropTypes.object),
+  // Tasks are a GeoJSON FeatureCollection
+  tasks: FeatureCollectionPropType,
   navigate: PropTypes.func,
   type: PropTypes.string,
-  tasksError: PropTypes.string,
+  tasksError: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]),
   projectLoading: PropTypes.bool,
 };
 
@@ -406,7 +430,7 @@ ProjectDetailLeft.propTypes = {
       shortDescription: PropTypes.string,
     }),
     projectId: PropTypes.number,
-    tasks: PropTypes.arrayOf(PropTypes.object),
+    tasks: FeatureCollectionPropType,
   }).isRequired,
   contributors: PropTypes.arrayOf(PropTypes.object),
   className: PropTypes.string,

frontend/src/components/projectDetail/timeline.js

@@ -5,19 +5,29 @@ import {
   PointElement,
   LinearScale,
   CategoryScale,
-  TimeScale,
+  TimeSeriesScale,
   Legend,
   Tooltip,
 } from 'chart.js';
 import { Line } from 'react-chartjs-2';
+import 'chartjs-adapter-date-fns';
 import { useIntl } from 'react-intl';
 
 import messages from './messages';
 import { formatTimelineData, formatTimelineTooltip } from '../../utils/formatChartJSData';
 import { CHART_COLOURS } from '../../config';
 import { useTimeDiff } from '../../hooks/UseTimeDiff';
+import { xAxisTimeSeries } from '../../utils/chart';
 
-ChartJS.register(LineElement, PointElement, LinearScale, CategoryScale, TimeScale, Legend, Tooltip);
+ChartJS.register(
+  LineElement,
+  PointElement,
+  LinearScale,
+  CategoryScale,
+  TimeSeriesScale,
+  Legend,
+  Tooltip,
+);
 
 export default function ProjectTimeline({ tasksByDay }: Object) {
   const intl = useIntl();
@@ -41,7 +51,10 @@ export default function ProjectTimeline({ tasksByDay }: Object) {
             callbacks: { label: (context) => formatTimelineTooltip(context, true) },
           },
         },
-        scales: { xAxes: [{ type: 'time', time: { unit: unit } }] },
+        scales: {
+          y: { ticks: { beginAtZero: true } },
+          x: { ...xAxisTimeSeries(unit) },
+        },
       }}
     />
   );

frontend/src/components/projectStats/contributorsStats.js

@@ -1,6 +1,15 @@
 import React from 'react';
-import { Chart as ChartJS, ArcElement, BarElement, CategoryScale, LinearScale } from 'chart.js';
 import { Doughnut, Bar } from 'react-chartjs-2';
+import {
+  Chart as ChartJS,
+  ArcElement,
+  BarElement,
+  CategoryScale,
+  Legend,
+  LinearScale,
+  Title,
+  Tooltip,
+} from 'chart.js';
 import { FormattedMessage, useIntl } from 'react-intl';
 
 import messages from './messages';
@@ -10,9 +19,8 @@ import { formatChartData, formatTooltip } from '../../utils/formatChartJSData';
 import { useContributorStats } from '../../hooks/UseContributorStats';
 import { StatsCardContent } from '../statsCard';
 
-ChartJS.register(ArcElement, BarElement, CategoryScale, LinearScale);
-
 export default function ContributorsStats({ contributors }) {
+  ChartJS.register(BarElement, CategoryScale, Legend, LinearScale, Title, Tooltip, ArcElement);
   const intl = useIntl();
   const stats = useContributorStats(contributors);
   const getUserLevelLabel = (level) => intl.formatMessage(userMessages[`mapperLevel${level}`]);

frontend/src/components/teamsAndOrgs/tasksStatsChart.js

@@ -6,7 +6,7 @@ import {
   BarElement,
   Tooltip,
   Legend,
-  TimeScale,
+  TimeSeriesScale,
 } from 'chart.js';
 import { Bar } from 'react-chartjs-2';
 import 'chartjs-adapter-date-fns';
@@ -16,8 +16,9 @@ import messages from '../projectDetail/messages';
 import { CHART_COLOURS } from '../../config';
 import { useTimeDiff } from '../../hooks/UseTimeDiff';
 import { formatTasksStatsData, formatTimelineTooltip } from '../../utils/formatChartJSData';
+import { xAxisTimeSeries } from '../../utils/chart';
 
-ChartJS.register(CategoryScale, LinearScale, BarElement, Tooltip, Legend, TimeScale);
+ChartJS.register(CategoryScale, LinearScale, BarElement, Tooltip, Legend, TimeSeriesScale);
 
 const TasksStatsChart = ({ stats }) => {
   const intl = useIntl();
@@ -39,21 +40,16 @@ const TasksStatsChart = ({ stats }) => {
       },
     },
     scales: {
-      yAxes: [
-        {
-          stacked: true,
-          ticks: {
-            beginAtZero: true,
-          },
+      y: {
+        stacked: true,
+        ticks: {
+          beginAtZero: true,
         },
-      ],
-      xAxes: [
-        {
-          stacked: true,
-          type: 'time',
-          time: { unit: unit },
-        },
-      ],
+      },
+      x: {
+        stacked: true,
+        ...xAxisTimeSeries(unit),
+      },
     },
   };
   return (

frontend/src/components/teamsAndOrgs/tests/tasksStats.test.js

@@ -7,6 +7,10 @@ import { tasksStats } from '../../../network/tests/mockData/tasksStats';
 import { TasksStats } from '../tasksStats';
 import userEvent from '@testing-library/user-event';
 
+// This is a late import in a React.lazy call; it takes awhile for date-fns to resolve, so we import it here manually.
+// In the event you remove it, please measure test times before ''and'' after removal.
+import '../../../utils/chart';
+
 jest.mock('react-chartjs-2', () => ({
   Bar: () => null,
 }));

frontend/src/network/tests/mockData/projects.js

@@ -323,6 +323,7 @@ export const taskDetail = (taskId) => ({
 export const projectComments = {
   chat: [
     {
+      id: 1,
       message:
         "<p>@happy_me we do want 'significant' roads that lead to houses. Rule of thumb I use for picking classification is the usage over condition/what it looks like. If it's the main 'path' to one or maybe several homes, I would pick service; even if a vehicle can't drive it, that can be reflected with additional tags, but the road still functions as access to the home(s).</p>",
       pictureUrl:
@@ -331,20 +332,23 @@ export const projectComments = {
       username: 'helnershingthapa',
     },
     {
+      id: 2,
       message: '<p>hello world</p>',
       pictureUrl:
         'https://www.openstreetmap.org/rails/active_storage/representations/redirect/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBBNXQ2Q3c9PSIsImV4cCI6bnVsbCwicHVyIjoiYmxvYl9pZCJ9fQ==--fe41f1b2a5d6cf492a7133f15c81f105dec06ff7/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaDdCem9MWm05eWJXRjBPZ2h3Ym1jNkZISmxjMmw2WlY5MGIxOXNhVzFwZEZzSGFXbHBhUT09IiwiZXhwIjpudWxsLCJwdXIiOiJ2YXJpYXRpb24ifX0=--058ac785867b32287d598a314311e2253bd879a3/unnamed.webp',
       timestamp: '2023-01-03T10:54:25.805150Z',
       username: 'helnershingthapa',
     },
     {
+      id: 3,
       message: '<p>asdadadasdasdasd</p>',
       pictureUrl:
         'https://www.openstreetmap.org/rails/active_storage/representations/redirect/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBBeFJheFE9PSIsImV4cCI6bnVsbCwicHVyIjoiYmxvYl9pZCJ9fQ==--a765e2377a288bccae85da6604300251d9de6d39/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaDdCem9MWm05eWJXRjBTU0lJYW5CbkJqb0dSVlE2RkhKbGMybDZaVjkwYjE5c2FXMXBkRnNIYVdscGFRPT0iLCJleHAiOm51bGwsInB1ciI6InZhcmlhdGlvbiJ9fQ==--1d22b8d446683a272d1a9ff04340453ca7c374b4/bitmoji.jpg',
       timestamp: '2022-10-19T09:32:52.231545Z',
       username: 'Hel Nershing Thapa',
     },
     {
+      id: 4,
       message:
         '<p><code>test of \ncode block\nhmmm\npreview showed it as a block\nand monospace font\nbut not indented</code></p>',
       pictureUrl:
@@ -353,6 +357,7 @@ export const projectComments = {
       username: 'wireguy',
     },
     {
+      id: 5,
       message:
         '<p><code>this is a code\nblock\nshould it\nbe indented\nby 4 space?\nminor...</code></p>',
       pictureUrl:

frontend/src/utils/chart.js

@@ -0,0 +1,24 @@
+import { enUS } from 'date-fns/locale';
+import { formatISO } from 'date-fns';
+/**
+ * x axis configuration common between this and {@link ../projectDetail/timeline.js}
+ * @param unit The base unit for the axis
+ * @typedef {import('chart.js').ScaleOptionsByType} ScaleOptionsByType
+ * @returns {ScaleOptionsByType} The options to use for x axis configuration
+ */
+function xAxisTimeSeries(unit) {
+  return {
+    type: 'timeseries',
+    adapters: { date: { locale: enUS } },
+    time: {
+      unit: unit,
+      tooltipFormat: enUS.formatLong.date,
+    },
+    ticks: {
+      source: 'labels',
+      callback: (value, index, ticks) => formatISO(ticks[index].value, { representation: 'date' }),
+    },
+  };
+}
+
+export { xAxisTimeSeries };

frontend/src/views/tests/project.test.js

@@ -24,6 +24,13 @@ import { setupFaultyHandlers } from '../../network/tests/server';
 import { projects } from '../../network/tests/mockData/projects';
 import { MemoryRouter, Route, Routes } from 'react-router-dom';
 
+// This is a late import in a React.lazy call; it takes awhile for date-fns to resolve, so we import it here manually.
+// In the event you remove it, please measure test times before ''and'' after removal.
+import '../../utils/chart';
+
+// scrollTo is not implemented by jsdom; mock to avoid warnings.
+window.scrollTo = jest.fn();
+
 test('CreateProject renders ProjectCreate', async () => {
   renderWithRouter(
     <QueryParamProvider adapter={ReactRouter6Adapter}>
@@ -221,27 +228,13 @@ describe('Project Detail Page', () => {
     Line: () => null,
   }));
 
-  it('should render component details', async () => {
-    act(() => {
-      store.dispatch({ type: 'SET_LOCALE', locale: 'es-AR' });
-    });
-    renderWithRouter(
-      <QueryClientProviders>
-        <ReduxIntlProviders>
-          <ProjectDetailPage />
-        </ReduxIntlProviders>
-      </QueryClientProviders>,
-    );
-    await waitFor(() => {
-      expect(screen.getByText(/sample project/i)).toBeInTheDocument();
-      expect(screen.getByText(/hello world/i)).toBeInTheDocument();
-    });
-  });
-
-  it('should display private project error message', async () => {
-    setupFaultyHandlers();
+  /**
+   * Set up a ProjectDetailPage given an initial entry; this avoids issues where there is no project id.
+   * @param {Array<string>} initialEntries The initial entries. This should be in the form of `[projects/:id]`.
+   */
+  function setup(initialEntries) {
     render(
-      <MemoryRouter initialEntries={['/projects/123']}>
+      <MemoryRouter initialEntries={initialEntries}>
         <Routes>
           <Route
             path="projects/:id"
@@ -256,6 +249,22 @@ describe('Project Detail Page', () => {
         </Routes>
       </MemoryRouter>,
     );
+  }
+
+  it('should render component details', async () => {
+    act(() => {
+      store.dispatch({ type: 'SET_LOCALE', locale: 'es-AR' });
+    });
+    setup(['/projects/123']);
+    await waitFor(() => {
+      expect(screen.getByText(/sample project/i)).toBeInTheDocument();
+      expect(screen.getByText(/hello world/i)).toBeInTheDocument();
+    });
+  });
+
+  it('should display private project error message', async () => {
+    setupFaultyHandlers();
+    setup(['/projects/123']);
 
     await waitFor(() =>
       expect(