Skip to content

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

Merged
jloh02 merged 4 commits into
masterfrom
scrapers/filter-inactive-entities
May 21, 2026
Merged

fix(scrapers): filter out inactive faculties and departments#4313
jloh02 merged 4 commits into
masterfrom
scrapers/filter-inactive-entities

Conversation

@ravern
Copy link
Copy Markdown
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
Copy Markdown

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 May 21, 2026 4:45am
nusmods-website Ignored Ignored Preview May 21, 2026 4:45am

Request Review

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 2, 2026

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4313      +/-   ##
==========================================
+ Coverage   54.52%   56.42%   +1.89%     
==========================================
  Files         274      317      +43     
  Lines        6076     6962     +886     
  Branches     1455     1679     +224     
==========================================
+ Hits         3313     3928     +615     
- Misses       2763     3034     +271     

☔ 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
Copy Markdown
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
Copy Markdown
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
Copy link
Copy Markdown
Member

@jloh02 jloh02 left a comment

Choose a reason for hiding this comment

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

Thanks Claude (and Ravern)!

@jloh02 jloh02 enabled auto-merge (squash) May 21, 2026 04:46
@jloh02 jloh02 merged commit bb17cc0 into master May 21, 2026
6 checks passed
@jloh02 jloh02 deleted the scrapers/filter-inactive-entities branch May 21, 2026 04:52
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.

2 participants