Add typeshare support for ChangelogItem type (#762)

Author: terror

Cargo.lock

@@ -2,6 +2,13 @@
  Generated by typeshare 1.13.3
 */
 
+export interface ChangelogItem {
+  number: number;
+  summary?: string;
+  url: string;
+  mergedAt: string;
+}
+
 export interface Instructor {
   name: string;
   nameNgrams?: string;

client/src/assets/changelog.json

@@ -0,0 +1,106 @@
+import { cleanup, render, screen, within } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import type { ReactNode } from 'react';
+import { HelmetProvider } from 'react-helmet-async';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+
+import type { ChangelogItem } from '../lib/types';
+
+const createItems = (count: number, month: number): ChangelogItem[] =>
+  Array.from({ length: count }, (_, index) => {
+    const number = month * 100 + index + 1;
+    return {
+      number,
+      summary: `Summary ${number}`,
+      url: `https://example.com/${number}`,
+      mergedAt: `2024-${String(month).padStart(2, '0')}-01T00:00:00Z`,
+    };
+  });
+
+vi.mock('../components/layout', () => ({
+  Layout: ({ children }: { children: ReactNode }) => (
+    <div data-testid='layout'>{children}</div>
+  ),
+}));
+
+const mockChangelog = {
+  'April 2024': createItems(11, 4),
+  'March 2024': createItems(2, 3),
+};
+
+const resetMockChangelog = () => {
+  mockChangelog['April 2024'] = createItems(11, 4);
+  mockChangelog['March 2024'] = createItems(2, 3);
+};
+
+vi.mock('../assets/changelog.json', () => ({
+  default: mockChangelog,
+}));
+
+const renderChangelog = async () => {
+  const { Changelog } = await import('./changelog');
+
+  return render(
+    <HelmetProvider>
+      <Changelog />
+    </HelmetProvider>
+  );
+};
+
+describe('Changelog page', () => {
+  beforeEach(() => {
+    resetMockChangelog();
+  });
+
+  afterEach(() => {
+    cleanup();
+  });
+
+  it('renders months in descending chronological order', async () => {
+    await renderChangelog();
+
+    const headings = screen.getAllByRole('heading', { level: 2 });
+
+    expect(headings.map((heading) => heading.textContent)).toEqual([
+      'April 2024',
+      'March 2024',
+    ]);
+  });
+
+  it('limits entries to five by default and expands when requested', async () => {
+    const user = userEvent.setup();
+    await renderChangelog();
+
+    const aprilSection = screen.getByText('April 2024').closest('div');
+    expect(aprilSection).toBeTruthy();
+
+    const withinApril = within(aprilSection as HTMLElement);
+    expect(withinApril.getAllByRole('link')).toHaveLength(5);
+
+    const toggleButton = withinApril.getByRole('button', { name: 'Show all' });
+
+    await user.click(toggleButton);
+    expect(withinApril.getAllByRole('link')).toHaveLength(11);
+    expect(toggleButton).toHaveTextContent('Show less');
+
+    await user.click(toggleButton);
+    expect(withinApril.getAllByRole('link')).toHaveLength(5);
+    expect(toggleButton).toHaveTextContent('Show all');
+  });
+
+  it('skips entries without a summary', async () => {
+    mockChangelog['March 2024'][1] = {
+      ...mockChangelog['March 2024'][1],
+      summary: undefined,
+    };
+
+    await renderChangelog();
+
+    const marchSection = screen.getByText('March 2024').closest('div');
+    expect(marchSection).toBeTruthy();
+
+    const withinMarch = within(marchSection as HTMLElement);
+    expect(withinMarch.getAllByRole('link')).toHaveLength(1);
+    expect(withinMarch.queryByText(/#302/)).not.toBeInTheDocument();
+  });
+});

client/src/lib/types.ts

@@ -3,7 +3,7 @@ import { Helmet } from 'react-helmet-async';
 
 import changelogItems from '../assets/changelog.json';
 import { Layout } from '../components/layout';
-import { ChangelogItem } from '../model/changelog-item';
+import type { ChangelogItem } from '../lib/types';
 
 const typedChangelogItems: Record<string, ChangelogItem[]> = changelogItems;
 
@@ -56,15 +56,29 @@ export const Changelog = () => {
           </p>
         </div>
         <div className='w-full max-w-4xl px-4 sm:px-6 lg:px-8'>
-          {sortedChangelogItems.map(([month, items]) => (
-            <div key={month} className='mb-8'>
-              <h2 className='text-2xl font-semibold text-gray-900 dark:text-gray-200'>
-                {month}
-              </h2>
-              {items
-                .slice(0, expandedMonths.includes(month) ? items.length : 5)
-                .map((item, index) => (
-                  <div key={index} className='mt-4'>
+          {sortedChangelogItems.map(([month, items]) => {
+            const sanitizedItems = items.filter(
+              (
+                item
+              ): item is ChangelogItem & {
+                summary: string;
+              } =>
+                typeof item.summary === 'string' &&
+                item.summary.trim().length > 0
+            );
+
+            const itemsToRender = sanitizedItems.slice(
+              0,
+              expandedMonths.includes(month) ? sanitizedItems.length : 5
+            );
+
+            return (
+              <div key={month} className='mb-8'>
+                <h2 className='text-2xl font-semibold text-gray-900 dark:text-gray-200'>
+                  {month}
+                </h2>
+                {itemsToRender.map((item) => (
+                  <div key={item.number} className='mt-4'>
                     <p className='text-lg text-gray-800 dark:text-gray-300'>
                       - {item.summary.replace('/^- /', '')} (
                       <a
@@ -79,16 +93,17 @@ export const Changelog = () => {
                     </p>
                   </div>
                 ))}
-              {items.length > 10 && (
-                <button
-                  onClick={() => toggleShowAll(month)}
-                  className='mt-4 underline dark:text-gray-400'
-                >
-                  {expandedMonths.includes(month) ? 'Show less' : 'Show all'}
-                </button>
-              )}
-            </div>
-          ))}
+                {sanitizedItems.length > 10 && (
+                  <button
+                    onClick={() => toggleShowAll(month)}
+                    className='mt-4 underline dark:text-gray-400'
+                  >
+                    {expandedMonths.includes(month) ? 'Show less' : 'Show all'}
+                  </button>
+                )}
+              </div>
+            );
+          })}
         </div>
       </div>
     </Layout>

client/src/model/changelog-item.ts

@@ -15,3 +15,4 @@ serde_json = "1.0.143"
 tokio = { version = "1.47.1", features = ["rt-multi-thread", "macros"] }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true }
+typeshare = { workspace = true }

client/src/pages/changelog.test.tsx

@@ -17,6 +17,7 @@ use {
   },
   tracing::info,
   tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt},
+  typeshare::typeshare,
 };
 
 const BASE_URL: &str = "https://github.com";
@@ -100,35 +101,40 @@ impl PullRequest<'_> {
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize, Eq, Hash, PartialEq)]
-struct Item {
-  number: u64,
-  summary: Option<String>,
-  url: String,
-  merged_at: DateTime<Utc>,
+#[serde(rename_all = "camelCase")]
+#[typeshare]
+pub struct ChangelogItem {
+  pub number: u32,
+  pub summary: Option<String>,
+  pub url: String,
+  #[serde(alias = "merged_at")]
+  #[typeshare(serialized_as = "String")]
+  pub merged_at: DateTime<Utc>,
 }
 
-impl Item {
+impl ChangelogItem {
   async fn try_from(
     pull_request: PullRequest<'_>,
     merged_at: DateTime<Utc>,
-  ) -> Result<Item> {
-    Ok(Item {
-      number: pull_request.number,
+    number: u32,
+  ) -> Result<ChangelogItem> {
+    Ok(ChangelogItem {
+      number,
       summary: pull_request.summary().await?,
       url: pull_request.url()?,
       merged_at,
     })
   }
 }
 
-type Entry = HashMap<String, Vec<Item>>;
+type Entry = HashMap<String, Vec<ChangelogItem>>;
 
 #[derive(Parser)]
 struct Arguments {
   #[clap(long, default_value = "../../client/src/assets/changelog.json")]
   output: PathBuf,
   #[clap(long, value_delimiter = ' ', num_args = 0..)]
-  regenerate: Vec<u64>,
+  regenerate: Vec<u32>,
   #[clap(long, default_value = "false")]
   regenerate_all: bool,
   #[clap(long, default_value = "mcgill.courses")]
@@ -166,9 +172,9 @@ impl Arguments {
           .flat_map(|(_, value)| {
             value.into_iter().map(|item| (item.number, item))
           })
-          .collect::<HashMap<u64, Item>>()
+          .collect::<HashMap<u32, ChangelogItem>>()
       })
-      .unwrap_or_else(|_| HashMap::new());
+      .unwrap_or_else(|_| HashMap::<u32, ChangelogItem>::new());
 
     let pull_requests = model
       .iter()
@@ -186,24 +192,24 @@ impl Arguments {
 
     for pull_request in pull_requests {
       if let Some(merged_at) = pull_request.merged_at {
+        let number = u32::try_from(pull_request.number).map_err(|_| {
+          anyhow!("pull request number {} exceeds u32", pull_request.number)
+        })?;
+
         let month = merged_at.format("%B %Y").to_string();
 
-        if let Some(item) = existing_items.get(&pull_request.number) {
-          if self.regenerate_all
-            || self.regenerate.contains(&pull_request.number)
-          {
-            grouped
-              .entry(month)
-              .or_default()
-              .push(Item::try_from(pull_request, merged_at).await?);
+        if let Some(item) = existing_items.get(&number) {
+          if self.regenerate_all || self.regenerate.contains(&number) {
+            grouped.entry(month).or_default().push(
+              ChangelogItem::try_from(pull_request, merged_at, number).await?,
+            );
           } else {
             grouped.entry(month).or_default().push(item.clone());
           }
         } else {
-          grouped
-            .entry(month)
-            .or_default()
-            .push(Item::try_from(pull_request, merged_at).await?);
+          grouped.entry(month).or_default().push(
+            ChangelogItem::try_from(pull_request, merged_at, number).await?,
+          );
         }
       }
     }