Skip to content

Auto-fill clipId for video recordings#924

Merged
abchugh merged 3 commits intomainfrom
add-auto-detect-clipId-in-coverage-editor
Apr 29, 2026
Merged

Auto-fill clipId for video recordings#924
abchugh merged 3 commits intomainfrom
add-auto-detect-clipId-in-coverage-editor

Conversation

@khushbu-25
Copy link
Copy Markdown
Collaborator

Implement auto-filling of clipId by matching lecture dates with recording dates.

Implement auto-filling of clipId by matching lecture dates with recording dates.
@khushbu-25 khushbu-25 requested review from XI4507 and abchugh April 28, 2026 11:53

useEffect(() => {
if (!courseId || !instanceId) return;
fetch(`/api/course-metadata/get-lecture?courseId=${courseId}&instanceId=${instanceId}`)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We dont use fetch. axios is more convenient.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

const fileData = fs.readFileSync(filePath, 'utf-8');
existingData = JSON.parse(fileData);
}
const backupDir = path.join(process.env.RECORDED_SYLLABUS_DIR!, 'backups');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is this?


const CoverageUpdateTab = ({ instanceId }: CoverageUpdateTabProps) => {
const router = useRouter();
const courseId = router.query.courseId as string;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

courseId is being fetched from the router query. Why do you need the instanceId to be passed on as a prop?
Both apporaches are fine but better to be consistent

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

const [timezone, setTimezone] = useState<string | undefined>(undefined);

const [notCoveredSections, setNotCoveredSections] = useState<string[]>([]);
const [clips, setClips] = useState<any[]>([]);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

dont use 'any'. Use proper types

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done


const fetchAllClips = async () => {
try {
const res = await axios.get(`/api/get-fau-series-clips/${seriesId}`);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We almost always create API 'spec' and dont make the API call directly. Do you know why?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done


useEffect(() => {
if (!courseId || !instanceId) return;
fetch(`/api/course-metadata/get-lecture?courseId=${courseId}&instanceId=${instanceId}`)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why fetch series id here when it is only used it the child component?

useEffect(() => {
if (!seriesId || clips.length === 0) return;

const lectureDate = dayjs(formData.timestamp_ms).format('YYYY-MM-DD');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do you need the same code in 2 places? Here and in handleEditDialogOpen ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

}

async function getSeriesClipsCached(seriesId: string) {
const cachedInfo = CACHED_SERIES_CLIPS.get(seriesId);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why did you choose this kind of caching?
Is it better to create a new API that caches fau.tv clips API instead of calling clips API directly from frontend (and perhaps using tanstack query for caching)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

@abchugh abchugh merged commit fceb760 into main Apr 29, 2026
1 check failed
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