Skip to content

fix(scrapers): filter out inactive faculties and departments#4313

Open
ravern wants to merge 3 commits intomasterfrom
scrapers/filter-inactive-entities
Open

fix(scrapers): filter out inactive faculties and departments#4313
ravern wants to merge 3 commits intomasterfrom
scrapers/filter-inactive-entities

Conversation

@ravern
Copy link
Member

@ravern ravern commented Mar 2, 2026

Summary

  • The new API returns all faculties and departments regardless of their active status, unlike the old API which accepted an eff_status: 'A' parameter for server-side filtering
  • Added client-side filtering by EffectiveStatus === 'A' for both faculties and departments after fetching (and after the 099 fallback), before the cleanFacultyDepartment mapping

Test plan

  • Verify that inactive faculties/departments are excluded from the output
  • Verify that the manually-added 099 faculty (which has EffectiveStatus: 'A') is retained
  • Verify that facultyDepartments.json output only contains active entries

🤖 Generated with Claude Code

The new API returns all faculties and departments regardless of status,
unlike the old API which accepted an eff_status parameter. Filter by
EffectiveStatus === 'A' after fetching to exclude inactive entries.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
nusmods-export Ignored Ignored Preview Mar 13, 2026 7:04am
nusmods-website Ignored Ignored Preview Mar 13, 2026 7:04am

Request Review

@codecov
Copy link

codecov bot commented Mar 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.67%. Comparing base (988c6fd) to head (40e748d).
⚠️ Report is 210 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4313      +/-   ##
==========================================
+ Coverage   54.52%   56.67%   +2.15%     
==========================================
  Files         274      308      +34     
  Lines        6076     6964     +888     
  Branches     1455     1682     +227     
==========================================
+ Hits         3313     3947     +634     
- Misses       2763     3017     +254     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 2, 2026

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change is a simple, well-placed filter that addresses the API behavior change. The logic is straightforward, type-safe, and preserves the manually-added 099 faculty.
  • No files require special attention

Important Files Changed

Filename Overview
scrapers/nus-v2/src/tasks/GetFacultyDepartment.ts Added client-side filtering to exclude inactive faculties and departments by EffectiveStatus === 'A'

Last reviewed commit: 24e3dca

@ravern
Copy link
Member Author

ravern commented Mar 2, 2026

This is actually a no-op in terms of data produced, since it looks like there are no inactive faculties/departments.

So the intention here is to just preserve functionality from previous version of scraper.

@ravern ravern requested a review from a team March 2, 2026 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant