diff --git a/app/controllers/admin/projects_controller.rb b/app/controllers/admin/projects_controller.rb index c3e64a25..d0c3c314 100644 --- a/app/controllers/admin/projects_controller.rb +++ b/app/controllers/admin/projects_controller.rb @@ -90,7 +90,10 @@ def toggle_staff_pick def restore authorize @project - @project.undiscard + unless @project.undiscard + return redirect_to admin_project_path(@project), alert: "Couldn't restore project: #{@project.errors.full_messages.to_sentence}" + end + audit!("project.restored", target: @project) redirect_to admin_project_path(@project), notice: "Project '#{@project.name}' has been restored." end @@ -882,7 +885,7 @@ def fine_guard_reason(project, params) approved_hours: capped_override_hours(project, params[:override_hours]) || project.devlog_hours, journal_only: project.devlog_mode.to_s != "git", uses_ai: project.uses_ai, - has_lapse: project.devlogs.any? { |d| d.lapse_url.present? }, + has_lapse: project.devlogs.any? { |d| d.lapse_urls.any? }, red_flags: project.red_flags || [] ) blocking = issues.select(&:blocking?) @@ -1148,7 +1151,7 @@ def serialize_devlog(devlog) content: devlog.content, time_spent: devlog.time_spent, time_hours: devlog.time_hours&.to_f, - lapse_url: devlog.lapse_url, + lapse_urls: devlog.lapse_urls, created_at: devlog.created_at.strftime("%b %d, %Y"), meets_requirements: devlog.meets_submission_requirements?, content_lost: devlog.content_lost, diff --git a/app/controllers/admin/reviews_controller.rb b/app/controllers/admin/reviews_controller.rb index 9c252c89..77e267dd 100644 --- a/app/controllers/admin/reviews_controller.rb +++ b/app/controllers/admin/reviews_controller.rb @@ -548,7 +548,7 @@ def serialize_devlog(devlog) content: devlog.content, time_spent: devlog.time_spent, time_hours: devlog.time_hours&.to_f, - lapse_url: devlog.lapse_url, + lapse_urls: devlog.lapse_urls, created_at: devlog.created_at.strftime("%b %d, %Y"), created_at_iso: devlog.created_at.iso8601, user_id: devlog.user_id, diff --git a/app/controllers/devlogs_controller.rb b/app/controllers/devlogs_controller.rb index 7bff624e..483243bc 100644 --- a/app/controllers/devlogs_controller.rb +++ b/app/controllers/devlogs_controller.rb @@ -22,7 +22,7 @@ def show content: @devlog.content, time_spent: @devlog.time_spent, time_hours: @devlog.time_hours&.to_f, - lapse_url: policy(@devlog).view_lapse_url? ? @devlog.lapse_url : nil, + lapse_urls: policy(@devlog).view_lapse_url? ? @devlog.lapse_urls : [], created_at: @devlog.created_at.strftime("%B %d, %Y"), user_id: @devlog.user_id, user_display_name: @devlog.user.display_name, @@ -130,9 +130,9 @@ def set_project def devlog_params if @project.devlog_mode == "git" - params.expect(devlog: [ :lapse_url ]) + params.expect(devlog: [ lapse_urls: [] ]) else - attrs = params.expect(devlog: [ :title, :content, :time_spent, :lapse_url ]) + attrs = params.expect(devlog: [ :title, :content, :time_spent, lapse_urls: [] ]) attrs[:time_spent] = normalize_time_spent(attrs[:time_spent]) attrs[:time_hours] = TimeSpentParser.parse(attrs[:time_spent]) attrs diff --git a/app/controllers/orphaned_lapse_links_controller.rb b/app/controllers/orphaned_lapse_links_controller.rb index 83717f59..799eec75 100644 --- a/app/controllers/orphaned_lapse_links_controller.rb +++ b/app/controllers/orphaned_lapse_links_controller.rb @@ -8,7 +8,7 @@ def relink devlog = @project.devlogs.find(params[:devlog_id]) ActiveRecord::Base.transaction do - devlog.update!(lapse_url: link.lapse_url) + devlog.update!(lapse_urls: devlog.lapse_urls + [ link.lapse_url ]) link.destroy! end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 60b899ae..de883e20 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -558,7 +558,9 @@ def set_project end def project_params - params.expect(project: [ :name, :subtitle, :repo_link, :tier, :devlog_mode, :linked_project_id, :uses_ai, :ai_usage, tags: [], hackatime_projects: [] ]) + fields = [ :name, :subtitle, :repo_link, :tier, :devlog_mode, :uses_ai, :ai_usage, tags: [], hackatime_projects: [] ] + fields.unshift(:linked_project_id) if action_name == "create" + params.expect(project: fields) end def parent_project_for_new @@ -717,7 +719,7 @@ def serialize_devlog(devlog) content: devlog.content, time_spent: devlog.time_spent, time_hours: devlog.time_hours&.to_f, - lapse_url: policy(devlog).view_lapse_url? ? devlog.lapse_url : nil, + lapse_urls: policy(devlog).view_lapse_url? ? devlog.lapse_urls : [], created_at: devlog.created_at.strftime("%B %d, %Y"), user_id: devlog.user_id, user_display_name: devlog.user.display_name, diff --git a/app/javascript/components/LapseUrlsInput.tsx b/app/javascript/components/LapseUrlsInput.tsx new file mode 100644 index 00000000..7829d40e --- /dev/null +++ b/app/javascript/components/LapseUrlsInput.tsx @@ -0,0 +1,59 @@ +const MAX_LAPSE_URLS = 10 + +export default function LapseUrlsInput({ + value, + onChange, + placeholder, + inputClassName, +}: { + value: string[] + onChange: (urls: string[]) => void + placeholder: string + inputClassName: string +}) { + const urls = value.length > 0 ? value : [''] + + function update(index: number, url: string) { + onChange(urls.map((u, i) => (i === index ? url : u))) + } + + function remove(index: number) { + onChange(urls.filter((_, i) => i !== index)) + } + + return ( +
+ {urls.map((url, index) => ( +
+ update(index, e.target.value)} + className={`flex-1 ${inputClassName}`} + placeholder={placeholder} + /> + {urls.length > 1 && ( + + )} +
+ ))} + {urls.length < MAX_LAPSE_URLS && ( + + )} +
+ ) +} diff --git a/app/javascript/components/TimeSpentHint.tsx b/app/javascript/components/TimeSpentHint.tsx new file mode 100644 index 00000000..2acc4b31 --- /dev/null +++ b/app/javascript/components/TimeSpentHint.tsx @@ -0,0 +1,18 @@ +import { formatHours, parseTimeSpent } from '@/lib/timeSpent' + +export default function TimeSpentHint({ value }: { value: string }) { + if (!value.trim()) return null + + const hours = parseTimeSpent(value) + if (hours === null) { + return ( +

Couldn't read this time — try something like 3h 30m or 3:30.

+ ) + } + + return ( +

+ Counted as {formatHours(hours)} ({hours} {hours === 1 ? 'hour' : 'hours'}) +

+ ) +} diff --git a/app/javascript/components/admin/review/ContentTabs.tsx b/app/javascript/components/admin/review/ContentTabs.tsx index 3c2a107e..5928797a 100644 --- a/app/javascript/components/admin/review/ContentTabs.tsx +++ b/app/javascript/components/admin/review/ContentTabs.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react' +import { Fragment, useState } from 'react' import Markdown from 'react-markdown' import rehypeRaw from 'rehype-raw' import rehypeSanitize from 'rehype-sanitize' @@ -202,19 +202,19 @@ export function ContentTabs({ )} - {entry.lapse_url && ( - <> + {entry.lapse_urls.map((url, i) => ( + · - Lapse ↗ + Lapse{entry.lapse_urls.length > 1 ? ` ${i + 1}` : ''} ↗ - - )} + + ))} {entry.content_lost && Content vanished} {!entry.content_lost && !entry.meets_requirements && Below requirements} {!isGitJournal && } diff --git a/app/javascript/components/admin/review/JournalDeflationTable.tsx b/app/javascript/components/admin/review/JournalDeflationTable.tsx index 3b85047e..f0d42696 100644 --- a/app/javascript/components/admin/review/JournalDeflationTable.tsx +++ b/app/javascript/components/admin/review/JournalDeflationTable.tsx @@ -121,7 +121,7 @@ export function JournalDeflationTable({ {entry.created_at} {isGroupProject && · {entry.user_display_name}} - {entry.lapse_url && ( + {entry.lapse_urls.length > 0 && ( · lapse diff --git a/app/javascript/components/admin/review/JournalDigestPanel.tsx b/app/javascript/components/admin/review/JournalDigestPanel.tsx index 0fbfdaae..f410fbb7 100644 --- a/app/javascript/components/admin/review/JournalDigestPanel.tsx +++ b/app/javascript/components/admin/review/JournalDigestPanel.tsx @@ -149,7 +149,7 @@ function WeekRow({ {item.entry_date} · {item.chars} chars {!item.has_image && ' · no image'} {item.backfilled && ` · written ${item.written_on}`} - {item.lapse_url && ' · timelapse'} + {item.lapse_urls.length > 0 && ' · timelapse'} (d.lapse_url ?? '').trim()).length + const timelapses = project.devlogs.reduce((sum, d) => sum + d.lapse_urls.length, 0) if (timelapses > 0) parts.push(`${timelapses} timelapse ${timelapses === 1 ? 'link' : 'links'} attached.`) } diff --git a/app/javascript/lib/timeSpent.ts b/app/javascript/lib/timeSpent.ts new file mode 100644 index 00000000..5b850eb9 --- /dev/null +++ b/app/javascript/lib/timeSpent.ts @@ -0,0 +1,31 @@ +const round2 = (n: number) => Math.round(n * 100) / 100 + +// Mirrors lib/time_spent_parser.rb so the preview matches what the server stores. +export function parseTimeSpent(input: string): number | null { + const value = input.trim().toLowerCase() + if (!value) return null + + let m = value.match(/(\d+(?:\.\d+)?)\s*h(?:ours?)?\s*[,\s]*(\d+(?:\.\d+)?)\s*m?(?:in(?:utes?)?)?/) + if (m) return round2(parseFloat(m[1]) + parseFloat(m[2]) / 60) + + m = value.match(/^(\d+):(\d+)$/) + if (m) return round2(parseInt(m[1], 10) + parseInt(m[2], 10) / 60) + + m = value.match(/(\d+(?:\.\d+)?)\s*h(?:ours?)?/) + if (m) return round2(parseFloat(m[1])) + + m = value.match(/(\d+(?:\.\d+)?)\s*m(?:in(?:utes?)?)?/) + if (m) return round2(parseFloat(m[1]) / 60) + + if (/^\d+(?:\.\d+)?$/.test(value)) return round2(parseFloat(value)) + + return null +} + +export function formatHours(hours: number): string { + const totalMinutes = Math.round(hours * 60) + const h = Math.floor(totalMinutes / 60) + const m = totalMinutes % 60 + if (h === 0) return `${m}m` + return m === 0 ? `${h}h` : `${h}h ${m}m` +} diff --git a/app/javascript/pages/Admin/Projects/Show.tsx b/app/javascript/pages/Admin/Projects/Show.tsx index adce3e81..28790dce 100644 --- a/app/javascript/pages/Admin/Projects/Show.tsx +++ b/app/javascript/pages/Admin/Projects/Show.tsx @@ -620,17 +620,18 @@ export default function AdminProjectsShow({ {entry.time_spent} )} - {entry.lapse_url && ( + {entry.lapse_urls.map((url, i) => ( - Lapse + Lapse{entry.lapse_urls.length > 1 ? ` ${i + 1}` : ''} - )} + ))} · {entry.created_at}
diff --git a/app/javascript/pages/Admin/Reviews/Show.tsx b/app/javascript/pages/Admin/Reviews/Show.tsx index 5290e0c1..747a83c4 100644 --- a/app/javascript/pages/Admin/Reviews/Show.tsx +++ b/app/javascript/pages/Admin/Reviews/Show.tsx @@ -379,7 +379,7 @@ export default function AdminReviewsShow({ approved < claimed - 0.001 ? `claimed ${claimed.toFixed(1)}h, approved ${approved.toFixed(1)}h` : `${claimed.toFixed(1)}h approved in full` - const lapse = e.lapse_url ? ` — timelapse: ${e.lapse_url}` : '' + const lapse = e.lapse_urls.length > 0 ? ` — timelapse: ${e.lapse_urls.join(', ')}` : '' return `${window.location.origin}/projects/${project.id}#devlog-${e.id} — ${e.created_at}: ${e.title} (${verdict})${lapse}` }) .join('\n'), @@ -403,7 +403,7 @@ export default function AdminReviewsShow({ approved_hours: approvedHours, repo_link: project.repo_link, devlog_count: project.devlogs.length, - timelapse_urls: [...new Set(project.devlogs.map((d) => (d.lapse_url ?? '').trim()).filter(Boolean))], + timelapse_urls: [...new Set(project.devlogs.flatMap((d) => d.lapse_urls))], public_url: `${origin}/projects/${project.id}`, admin_url: `${origin}/admin/projects/${project.id}`, time_summary: timeSummary, @@ -578,7 +578,7 @@ export default function AdminReviewsShow({ } }, [project.id, justificationPreview, approvedHours, track]) - const hasLapse = useMemo(() => project.devlogs.some((d) => (d.lapse_url ?? '').trim().length > 0), [project.devlogs]) + const hasLapse = useMemo(() => project.devlogs.some((d) => d.lapse_urls.length > 0), [project.devlogs]) // Runs on every keystroke against the rule table the server sent, so the // reviewer sees the problem while writing. The server re-runs the same rules diff --git a/app/javascript/pages/Devlogs/Show.tsx b/app/javascript/pages/Devlogs/Show.tsx index 75e4df22..8ffbfc61 100644 --- a/app/javascript/pages/Devlogs/Show.tsx +++ b/app/javascript/pages/Devlogs/Show.tsx @@ -18,7 +18,7 @@ interface Props { content: string time_spent: string | null time_hours: number | null - lapse_url: string | null + lapse_urls: string[] created_at: string } can_edit: boolean @@ -38,7 +38,7 @@ export default function DevlogsShow({ project, devlog }: Props) {
- {(devlog.time_spent || devlog.lapse_url) && ( + {(devlog.time_spent || devlog.lapse_urls.length > 0) && (
{devlog.time_spent && ( @@ -46,17 +46,18 @@ export default function DevlogsShow({ project, devlog }: Props) { {devlog.time_spent} )} - {devlog.lapse_url && ( + {devlog.lapse_urls.map((url, i) => ( videocam - Lapse + Lapse{devlog.lapse_urls.length > 1 ? ` ${i + 1}` : ''} - )} + ))}
)} diff --git a/app/javascript/pages/Projects/Show.tsx b/app/javascript/pages/Projects/Show.tsx index eda29e7e..8d3bf06b 100644 --- a/app/javascript/pages/Projects/Show.tsx +++ b/app/javascript/pages/Projects/Show.tsx @@ -9,6 +9,8 @@ import { formatCoinRate } from '@/lib/tiers' import ReviewTimeline, { type ReviewEvent } from '@/components/ReviewTimeline' import HackatimePicker from '@/components/HackatimePicker' import CoverImageCropper, { type CropRegion } from '@/components/CoverImageCropper' +import LapseUrlsInput from '@/components/LapseUrlsInput' +import TimeSpentHint from '@/components/TimeSpentHint' import MacondoImportPrompt from '@/components/MacondoImportPrompt' function isSafeUrl(url: string | null): boolean { @@ -103,7 +105,7 @@ interface DevlogEntry { content: string time_spent: string | null time_hours: number | null - lapse_url: string | null + lapse_urls: string[] created_at: string user_id: number user_display_name: string @@ -404,7 +406,7 @@ export default function ProjectsShow({ title: '', content: '', time_spent: '', - lapse_url: '', + lapse_urls: [] as string[], }) const devlogDraftKey = `forge:devlog-draft:${project.id}` @@ -419,9 +421,9 @@ export default function ProjectsShow({ title: draft.title || '', content: draft.content || '', time_spent: draft.time_spent || '', - lapse_url: draft.lapse_url || '', + lapse_urls: Array.isArray(draft.lapse_urls) ? draft.lapse_urls : [], } - if (next.title || next.content || next.time_spent || next.lapse_url) { + if (next.title || next.content || next.time_spent || next.lapse_urls.some(Boolean)) { devlogForm.setData(next) setShowDevlogForm(true) } @@ -435,7 +437,7 @@ export default function ProjectsShow({ if (!devlogDraftRestored.current) return const d = devlogForm.data try { - if (d.title || d.content || d.time_spent || d.lapse_url) { + if (d.title || d.content || d.time_spent || d.lapse_urls.some(Boolean)) { localStorage.setItem(devlogDraftKey, JSON.stringify(d)) } else { localStorage.removeItem(devlogDraftKey) @@ -449,7 +451,7 @@ export default function ProjectsShow({ title: '', content: '', time_spent: '', - lapse_url: '', + lapse_urls: [] as string[], }) function startEditDevlog(entry: DevlogEntry) { @@ -458,7 +460,7 @@ export default function ProjectsShow({ title: entry.title, content: entry.content, time_spent: entry.time_spent || '', - lapse_url: entry.lapse_url || '', + lapse_urls: entry.lapse_urls, }) } @@ -479,11 +481,11 @@ export default function ProjectsShow({ } const [editingLapseId, setEditingLapseId] = useState(null) - const lapseForm = useForm({ lapse_url: '' }) + const lapseForm = useForm({ lapse_urls: [] as string[] }) function startEditLapse(entry: DevlogEntry) { setEditingLapseId(entry.id) - lapseForm.setData('lapse_url', entry.lapse_url || '') + lapseForm.setData('lapse_urls', entry.lapse_urls) } function cancelEditLapse() { @@ -1457,14 +1459,13 @@ export default function ProjectsShow({ ))}
)} + lapseForm.setData('lapse_urls', urls)} + inputClassName="bg-[#0e0e0e] border-none px-3 py-2 text-[#e5e2e1] focus:ring-1 focus:ring-[#ca5924]/30 placeholder:text-stone-600 text-xs" + placeholder="https://lapse.hackclub.com/timelapse" + />
- lapseForm.setData('lapse_url', e.target.value)} - className="flex-1 bg-[#0e0e0e] border-none px-3 py-2 text-[#e5e2e1] focus:ring-1 focus:ring-[#ca5924]/30 placeholder:text-stone-600 text-xs" - placeholder="https://lapse.hackclub.com/timelapse" - /> )}
@@ -1654,17 +1656,17 @@ export default function ProjectsShow({ className="w-full bg-[#0e0e0e] border-none px-4 py-3 text-[#e5e2e1] focus:ring-1 focus:ring-[#ca5924]/30 placeholder:text-stone-600 text-sm" placeholder="e.g. 3 hours" /> +
- devlogForm.setData('lapse_url', e.target.value)} - className="w-full bg-[#0e0e0e] border-none px-4 py-3 text-[#e5e2e1] focus:ring-1 focus:ring-[#ca5924]/30 placeholder:text-stone-600 text-sm" + devlogForm.setData('lapse_urls', urls)} + inputClassName="w-full bg-[#0e0e0e] border-none px-4 py-3 text-[#e5e2e1] focus:ring-1 focus:ring-[#ca5924]/30 placeholder:text-stone-600 text-sm" placeholder="https://lapse.hackclub.com/timelapse/create" />

@@ -1813,10 +1815,11 @@ export default function ProjectsShow({ className="w-full bg-[#0e0e0e] border-none px-4 py-3 text-[#e5e2e1] focus:ring-1 focus:ring-[#ca5924]/30 placeholder:text-stone-600 text-sm" placeholder="e.g. 3 or 3 hours" /> +

{orphaned_lapse_links.length > 0 && ( @@ -1833,11 +1836,10 @@ export default function ProjectsShow({ ))}
)} - editDevlogForm.setData('lapse_url', e.target.value)} - className="w-full bg-[#0e0e0e] border-none px-4 py-3 text-[#e5e2e1] focus:ring-1 focus:ring-[#ca5924]/30 placeholder:text-stone-600 text-sm" + editDevlogForm.setData('lapse_urls', urls)} + inputClassName="w-full bg-[#0e0e0e] border-none px-4 py-3 text-[#e5e2e1] focus:ring-1 focus:ring-[#ca5924]/30 placeholder:text-stone-600 text-sm" placeholder="https://lapse.hackclub.com/timelapse" /> @@ -1885,17 +1887,18 @@ export default function ProjectsShow({ {entry.time_spent} )} - {entry.lapse_url && ( + {entry.lapse_urls.map((url, i) => ( videocam - Lapse + Lapse{entry.lapse_urls.length > 1 ? ` ${i + 1}` : ''} - )} + ))} {entry.can_edit && ( diff --git a/app/javascript/types/index.ts b/app/javascript/types/index.ts index a6cecbbe..00feb6cb 100644 --- a/app/javascript/types/index.ts +++ b/app/javascript/types/index.ts @@ -266,7 +266,7 @@ export interface AdminDevlog { content: string time_spent: string | null time_hours: number | null - lapse_url: string | null + lapse_urls: string[] created_at: string meets_requirements: boolean content_lost: boolean diff --git a/app/jobs/sync_journal_job.rb b/app/jobs/sync_journal_job.rb index 9135d5c6..5e63ea3b 100644 --- a/app/jobs/sync_journal_job.rb +++ b/app/jobs/sync_journal_job.rb @@ -56,11 +56,11 @@ def perform(project_id, clear: false) preserved_lapse_urls = {} if clear - project.devlogs.where.not(lapse_url: [ nil, "" ]).find_each do |d| + project.devlogs.where("cardinality(lapse_urls) > 0").find_each do |d| if current_titles.include?(d.title) - preserved_lapse_urls[d.title] = d.lapse_url + preserved_lapse_urls[d.title] = d.lapse_urls else - stash_orphaned_lapse_link!(project, d.title, d.lapse_url) + stash_orphaned_lapse_links!(project, d.title, d.lapse_urls) end end project.devlogs.delete_all @@ -106,13 +106,13 @@ def perform(project_id, clear: false) end if clear - preserved_lapse_urls.each do |title, lapse_url| - project.devlogs.where(title: title).update_all(lapse_url: lapse_url) + preserved_lapse_urls.each do |title, lapse_urls| + project.devlogs.where(title: title).update_all(lapse_urls: lapse_urls) end else - project.devlogs.where.not(lapse_url: [ nil, "" ]).where.not(title: current_titles).find_each do |d| - stash_orphaned_lapse_link!(project, d.title, d.lapse_url) - d.update_column(:lapse_url, nil) + project.devlogs.where("cardinality(lapse_urls) > 0").where.not(title: current_titles).find_each do |d| + stash_orphaned_lapse_links!(project, d.title, d.lapse_urls) + d.update_column(:lapse_urls, []) end end @@ -123,10 +123,26 @@ def perform(project_id, clear: false) private - def stash_orphaned_lapse_link!(project, title, lapse_url) - link = OrphanedLapseLink.find_or_initialize_by(project: project, title: title) - link.lapse_url = lapse_url - link.save! + def stash_orphaned_lapse_links!(project, title, lapse_urls) + urls = lapse_urls.uniq + return if urls.empty? + + existing_urls = OrphanedLapseLink.where(project: project, title: title, lapse_url: urls).pluck(:lapse_url) + missing_urls = urls - existing_urls + return if missing_urls.empty? + + now = Time.current + rows = missing_urls.map do |lapse_url| + { + project_id: project.id, + title: title, + lapse_url: lapse_url, + created_at: now, + updated_at: now + } + end + + OrphanedLapseLink.insert_all!(rows) end def parse_repo_url(url) diff --git a/app/lib/justification_template.rb b/app/lib/justification_template.rb index 4af72134..083d57fa 100644 --- a/app/lib/justification_template.rb +++ b/app/lib/justification_template.rb @@ -128,7 +128,7 @@ def self.supporting_evidence(project, fields) "- Public project page: #{public_project_url(project)}", "- Devlogs: #{count} #{count == 1 ? 'entry' : 'entries'}" ] - lapse_urls = project.devlogs.map(&:lapse_url).map(&:presence).compact.uniq + lapse_urls = project.devlogs.flat_map(&:lapse_urls).uniq lines << "- Timelapse links: #{lapse_urls.join(', ')}" if lapse_urls.any? fields[:evidence].to_s.split("\n").map(&:strip).reject(&:blank?).each { |line| lines << "- #{line}" } lines.join("\n") diff --git a/app/models/devlog.rb b/app/models/devlog.rb index 87f85387..bda85283 100644 --- a/app/models/devlog.rb +++ b/app/models/devlog.rb @@ -7,7 +7,7 @@ # content :text # content_lost :boolean default(FALSE), not null # entry_date :date -# lapse_url :string +# lapse_urls :string default([]), not null, is an Array # review_feedback :text # reviewed_at :datetime # status :integer default("draft"), not null @@ -35,6 +35,9 @@ # fk_rails_... (user_id => users.id) # class Devlog < ApplicationRecord + MAX_LAPSE_URLS = 10 + LAPSE_URL_FORMAT = /\Ahttps?:\/\/\S+\z/i + has_paper_trail belongs_to :project @@ -52,7 +55,9 @@ class Devlog < ApplicationRecord validates :title, presence: true validates :content, presence: true - validates :lapse_url, format: { with: /\Ahttps?:\/\/\S+\z/i, message: "must be a valid URL" }, allow_blank: true + validate :lapse_urls_must_be_valid + + normalizes :lapse_urls, with: ->(urls) { Array(urls).map { |u| u.to_s.strip }.compact_blank.uniq }, apply_to_nil: true default_scope { order(id: :desc) } @@ -83,6 +88,11 @@ def requirement_validation_details private + def lapse_urls_must_be_valid + errors.add(:lapse_urls, "can't have more than #{MAX_LAPSE_URLS} links") if lapse_urls.size > MAX_LAPSE_URLS + errors.add(:lapse_urls, "must be valid URLs") unless lapse_urls.all? { |u| u.match?(LAPSE_URL_FORMAT) } + end + def recredit_streak_day return if user.blank? diff --git a/app/models/project.rb b/app/models/project.rb index 688161c2..e722c9d7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -68,7 +68,7 @@ # index_projects_on_discarded_at (discarded_at) # index_projects_on_flagged_by_id (flagged_by_id) # index_projects_on_flagged_for_review_at (flagged_for_review_at) -# index_projects_on_linked_project_id_for_build_reviews (linked_project_id) UNIQUE WHERE (build_review = true) +# index_projects_on_linked_project_id_for_build_reviews (linked_project_id) UNIQUE WHERE ((build_review = true) AND (discarded_at IS NULL)) # index_projects_on_macondo_project_id (macondo_project_id) UNIQUE # index_projects_on_parent_project_id (parent_project_id) # index_projects_on_requirements_checked_at (requirements_checked_at) @@ -148,6 +148,7 @@ class Project < ApplicationRecord validates :tier, inclusion: { in: ALL_TIERS } validate :build_review_consistency validate :linked_project_must_be_approved_and_owned + validates :linked_project_id, uniqueness: { conditions: -> { kept.build_reviews }, message: "already has a build review" }, allow_nil: true, if: -> { build_review? && !discarded? } validate :parent_project_must_be_owned_top_level_project validate :subproject_tier_within_parent diff --git a/app/services/journal_digest.rb b/app/services/journal_digest.rb index d20b4e25..41d6c699 100644 --- a/app/services/journal_digest.rb +++ b/app/services/journal_digest.rb @@ -28,7 +28,7 @@ def build(project) "last_entry_on" => date_of(entries.last)&.to_s, "span_days" => span_days(entries), "with_images" => entries.count { |e| image?(e) }, - "with_lapse" => entries.count { |e| e.lapse_url.present? }, + "with_lapse" => entries.count { |e| e.lapse_urls.any? }, "weeks" => weeks, "signals" => signals(entries, weeks) } @@ -44,7 +44,7 @@ def group_by_week(entries) "entries" => group.size, "hours" => group.sum { |e| e.parsed_hours.to_f }.round(1), "with_images" => group.count { |e| image?(e) }, - "lapse_urls" => group.filter_map { |e| e.lapse_url.presence }.uniq, + "lapse_urls" => group.flat_map(&:lapse_urls).uniq, "items" => group.map { |e| item(e) } } end @@ -58,7 +58,7 @@ def item(entry) "hours" => entry.parsed_hours.to_f.round(2), "chars" => entry.content.to_s.strip.length, "has_image" => image?(entry), - "lapse_url" => entry.lapse_url.presence, + "lapse_urls" => entry.lapse_urls, "written_on" => entry.created_at.to_date.to_s, "backfilled" => backfilled?(entry), "outlier" => entry.parsed_hours.to_f > OUTLIER_HOURS diff --git a/db/migrate/20261011014347_scope_build_review_link_index_to_kept_projects.rb b/db/migrate/20261011014347_scope_build_review_link_index_to_kept_projects.rb new file mode 100644 index 00000000..5ab96a32 --- /dev/null +++ b/db/migrate/20261011014347_scope_build_review_link_index_to_kept_projects.rb @@ -0,0 +1,10 @@ +class ScopeBuildReviewLinkIndexToKeptProjects < ActiveRecord::Migration[8.1] + def change + remove_index :projects, :linked_project_id, unique: true, + where: "build_review = true", + name: "index_projects_on_linked_project_id_for_build_reviews" + add_index :projects, :linked_project_id, unique: true, + where: "build_review = true AND discarded_at IS NULL", + name: "index_projects_on_linked_project_id_for_build_reviews" + end +end diff --git a/db/migrate/20261014103030_change_devlogs_lapse_url_to_lapse_urls.rb b/db/migrate/20261014103030_change_devlogs_lapse_url_to_lapse_urls.rb new file mode 100644 index 00000000..8a9062e1 --- /dev/null +++ b/db/migrate/20261014103030_change_devlogs_lapse_url_to_lapse_urls.rb @@ -0,0 +1,13 @@ +class ChangeDevlogsLapseUrlToLapseUrls < ActiveRecord::Migration[8.1] + def up + add_column :devlogs, :lapse_urls, :string, array: true, default: [], null: false + execute "UPDATE devlogs SET lapse_urls = ARRAY[lapse_url] WHERE lapse_url IS NOT NULL AND lapse_url <> ''" + remove_column :devlogs, :lapse_url + end + + def down + add_column :devlogs, :lapse_url, :string + execute "UPDATE devlogs SET lapse_url = lapse_urls[1]" + remove_column :devlogs, :lapse_urls + end +end diff --git a/db/schema.rb b/db/schema.rb index c006ea35..d4fb60d8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_10_12_104041) do +ActiveRecord::Schema[8.1].define(version: 2026_10_14_103030) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -178,7 +178,7 @@ t.boolean "content_lost", default: false, null: false t.datetime "created_at", null: false t.date "entry_date" - t.string "lapse_url" + t.string "lapse_urls", default: [], null: false, array: true t.bigint "project_id", null: false t.text "review_feedback" t.datetime "reviewed_at" @@ -377,7 +377,7 @@ t.index ["discarded_at"], name: "index_projects_on_discarded_at" t.index ["flagged_by_id"], name: "index_projects_on_flagged_by_id" t.index ["flagged_for_review_at"], name: "index_projects_on_flagged_for_review_at" - t.index ["linked_project_id"], name: "index_projects_on_linked_project_id_for_build_reviews", unique: true, where: "(build_review = true)" + t.index ["linked_project_id"], name: "index_projects_on_linked_project_id_for_build_reviews", unique: true, where: "((build_review = true) AND (discarded_at IS NULL))" t.index ["macondo_project_id"], name: "index_projects_on_macondo_project_id", unique: true t.index ["parent_project_id"], name: "index_projects_on_parent_project_id" t.index ["requirements_checked_at"], name: "index_projects_on_requirements_checked_at" diff --git a/lib/tasks/audit_fabricated_devlogs.rake b/lib/tasks/audit_fabricated_devlogs.rake index 309e4d30..7b84d53f 100644 --- a/lib/tasks/audit_fabricated_devlogs.rake +++ b/lib/tasks/audit_fabricated_devlogs.rake @@ -79,7 +79,7 @@ namespace :devlogs do puts " orphan devlogs: #{r[:orphans].size} headings in journal: #{r[:heading_count]} hours: #{r[:hours].round(2)}" puts " journal format: #{r[:journal_format]}" r[:orphans].each do |d| - flag = d.lapse_url.present? ? " [has lapse_url]" : "" + flag = d.lapse_urls.any? ? " [has lapse_urls]" : "" puts " ##{d.id} #{d.entry_date} #{d.time_hours.to_f.round(2)}h #{d.title.truncate(60).inspect}#{flag}" end puts diff --git a/lib/tasks/restore_build_review_links.rake b/lib/tasks/restore_build_review_links.rake new file mode 100644 index 00000000..5d608abc --- /dev/null +++ b/lib/tasks/restore_build_review_links.rake @@ -0,0 +1,44 @@ +namespace :projects do + desc "Restore build review links cleared by project edits and mark linked projects built. DRY_RUN=1 to preview." + task restore_build_review_links: :environment do + dry_run = ENV["DRY_RUN"].present? + restored = 0 + marked_built = 0 + + Project.kept.where(build_review: true, linked_project_id: nil).find_each do |review| + previous_id = review.versions.reorder(created_at: :desc, id: :desc).lazy.filter_map { |v| v.reify&.linked_project_id }.first + next unless previous_id + + target = Project.kept.find_by(id: previous_id, user_id: review.user_id, build_review: false) + if target.nil? + puts "✗ Build review #{review.id}: previous link #{previous_id} no longer valid" + next + end + if Project.kept.where(build_review: true, linked_project_id: target.id).exists? + puts "✗ Build review #{review.id}: project #{target.id} already has another build review" + next + end + + review.update_column(:linked_project_id, target.id) unless dry_run + restored += 1 + puts "✓ Build review #{review.id}: relinked to project #{target.id} (#{target.name})" + end + + Project.kept.where(build_review: true, status: :approved).where.not(linked_project_id: nil).includes(:linked_project).find_each do |review| + target = review.linked_project + next if target.nil? || target.built? + + unless dry_run + unless target.update(built_at: review.reviewed_at || Time.current) + puts "✗ Project #{target.id}: #{target.errors.full_messages.join(', ')}" + next + end + AuditEvent.create!(action: "project.marked_built", target: target, target_label: target.name, metadata: { via: "build_review_backfill", build_review_id: review.id }) + end + marked_built += 1 + puts "✓ Project #{target.id} (#{target.name}): marked built via build review #{review.id}" + end + + puts "\n#{dry_run ? '[DRY RUN] ' : ''}Relinked: #{restored}, marked built: #{marked_built}" + end +end diff --git a/test/controllers/devlogs_lapse_url_test.rb b/test/controllers/devlogs_lapse_url_test.rb index 649e7c4a..06213015 100644 --- a/test/controllers/devlogs_lapse_url_test.rb +++ b/test/controllers/devlogs_lapse_url_test.rb @@ -28,7 +28,7 @@ def sign_in_as(user) setup do @owner = make_user @project = @owner.projects.create!(name: "Macropad", tier: "tier_4", status: :draft, devlog_mode: "website") - @devlog = @project.devlogs.create!(user: @owner, title: "Entry", content: "Did things", lapse_url: "https://lapse.hackclub.com/timelapse/abc") + @devlog = @project.devlogs.create!(user: @owner, title: "Entry", content: "Did things", lapse_urls: [ "https://lapse.hackclub.com/timelapse/abc" ]) end test "an anonymous visitor does not see the lapse_url on the project page" do @@ -36,7 +36,7 @@ def sign_in_as(user) assert_response :success entry = inertia.props[:devlogs].find { |d| d[:id] == @devlog.id } - assert_nil entry[:lapse_url] + assert_empty entry[:lapse_urls] end test "a random signed-in user does not see the lapse_url" do @@ -45,7 +45,7 @@ def sign_in_as(user) get project_path(@project) entry = inertia.props[:devlogs].find { |d| d[:id] == @devlog.id } - assert_nil entry[:lapse_url] + assert_empty entry[:lapse_urls] end test "the project owner sees the lapse_url" do @@ -54,7 +54,7 @@ def sign_in_as(user) get project_path(@project) entry = inertia.props[:devlogs].find { |d| d[:id] == @devlog.id } - assert_equal @devlog.lapse_url, entry[:lapse_url] + assert_equal @devlog.lapse_urls, entry[:lapse_urls] end test "an admin sees the lapse_url" do @@ -63,7 +63,7 @@ def sign_in_as(user) get project_path(@project) entry = inertia.props[:devlogs].find { |d| d[:id] == @devlog.id } - assert_equal @devlog.lapse_url, entry[:lapse_url] + assert_equal @devlog.lapse_urls, entry[:lapse_urls] end test "a reviewer sees the lapse_url" do @@ -72,14 +72,14 @@ def sign_in_as(user) get project_path(@project) entry = inertia.props[:devlogs].find { |d| d[:id] == @devlog.id } - assert_equal @devlog.lapse_url, entry[:lapse_url] + assert_equal @devlog.lapse_urls, entry[:lapse_urls] end test "the single devlog page also hides the lapse_url from unauthorized viewers" do get project_devlog_path(@project, @devlog) assert_response :success - assert_nil inertia.props[:devlog][:lapse_url] + assert_empty inertia.props[:devlog][:lapse_urls] end test "a git-mode devlog update can only change the lapse_url, even if other fields are submitted" do @@ -88,14 +88,14 @@ def sign_in_as(user) sign_in_as(@owner) patch project_devlog_path(@project, @devlog), params: { - devlog: { title: "Hijacked", content: "Hijacked content", time_spent: "99 hours", lapse_url: "https://lapse.hackclub.com/timelapse/new" } + devlog: { title: "Hijacked", content: "Hijacked content", time_spent: "99 hours", lapse_urls: [ "https://lapse.hackclub.com/timelapse/new" ] } } @devlog.reload assert_equal "Original", @devlog.title assert_equal "Original content", @devlog.content assert_equal "1 hour", @devlog.time_spent - assert_equal "https://lapse.hackclub.com/timelapse/new", @devlog.lapse_url + assert_equal [ "https://lapse.hackclub.com/timelapse/new" ], @devlog.lapse_urls end test "a website-mode devlog update can still change all fields" do @@ -103,11 +103,11 @@ def sign_in_as(user) content = "Updated content describing what was built in plenty of detail. " * 3 + "![screenshot](https://example.com/a.png)" patch project_devlog_path(@project, @devlog), params: { - devlog: { title: "Updated", content: content, time_spent: "2 hours", lapse_url: "https://lapse.hackclub.com/timelapse/new" } + devlog: { title: "Updated", content: content, time_spent: "2 hours", lapse_urls: [ "https://lapse.hackclub.com/timelapse/new" ] } } @devlog.reload assert_equal "Updated", @devlog.title - assert_equal "https://lapse.hackclub.com/timelapse/new", @devlog.lapse_url + assert_equal [ "https://lapse.hackclub.com/timelapse/new" ], @devlog.lapse_urls end end diff --git a/test/controllers/orphaned_lapse_links_controller_test.rb b/test/controllers/orphaned_lapse_links_controller_test.rb index 3c26d834..f764dfed 100644 --- a/test/controllers/orphaned_lapse_links_controller_test.rb +++ b/test/controllers/orphaned_lapse_links_controller_test.rb @@ -38,7 +38,7 @@ def sign_in_as(user) patch relink_project_orphaned_lapse_link_path(@project, @link), params: { devlog_id: @devlog.id } assert_redirected_to project_path(@project) - assert_equal "https://lapse.hackclub.com/timelapse/old", @devlog.reload.lapse_url + assert_equal [ "https://lapse.hackclub.com/timelapse/old" ], @devlog.reload.lapse_urls assert_not OrphanedLapseLink.exists?(@link.id) end @@ -55,7 +55,7 @@ def sign_in_as(user) sign_in_as(make_user) patch relink_project_orphaned_lapse_link_path(@project, @link), params: { devlog_id: @devlog.id } - assert_nil @devlog.reload.lapse_url + assert_empty @devlog.reload.lapse_urls assert OrphanedLapseLink.exists?(@link.id) delete project_orphaned_lapse_link_path(@project, @link) @@ -67,6 +67,6 @@ def sign_in_as(user) patch relink_project_orphaned_lapse_link_path(@project, @link), params: { devlog_id: @devlog.id } - assert_equal "https://lapse.hackclub.com/timelapse/old", @devlog.reload.lapse_url + assert_equal [ "https://lapse.hackclub.com/timelapse/old" ], @devlog.reload.lapse_urls end end diff --git a/test/fixtures/projects.yml b/test/fixtures/projects.yml index b76d3516..97584f4a 100644 --- a/test/fixtures/projects.yml +++ b/test/fixtures/projects.yml @@ -70,7 +70,7 @@ # index_projects_on_discarded_at (discarded_at) # index_projects_on_flagged_by_id (flagged_by_id) # index_projects_on_flagged_for_review_at (flagged_for_review_at) -# index_projects_on_linked_project_id_for_build_reviews (linked_project_id) UNIQUE WHERE (build_review = true) +# index_projects_on_linked_project_id_for_build_reviews (linked_project_id) UNIQUE WHERE ((build_review = true) AND (discarded_at IS NULL)) # index_projects_on_macondo_project_id (macondo_project_id) UNIQUE # index_projects_on_parent_project_id (parent_project_id) # index_projects_on_requirements_checked_at (requirements_checked_at) diff --git a/test/jobs/sync_journal_job_test.rb b/test/jobs/sync_journal_job_test.rb index 3348a795..d3b18e51 100644 --- a/test/jobs/sync_journal_job_test.rb +++ b/test/jobs/sync_journal_job_test.rb @@ -141,42 +141,42 @@ def run_job(journal, commit_date: nil, clear: false) assert_not_nil @project.reload.journal_synced_at end - test "a normal resync never touches an existing lapse_url" do + test "a normal resync never touches an existing lapse_urls" do run_job("# Wiring\n\n**Total time spent: 30m**\n", commit_date: @today) devlog = Devlog.find_by(project: @project, title: "Wiring") - devlog.update!(lapse_url: "https://lapse.hackclub.com/timelapse/abc") + devlog.update!(lapse_urls: [ "https://lapse.hackclub.com/timelapse/abc" ]) run_job("# Wiring\n\n**Total time spent: 2h**\n", commit_date: @today) - assert_equal "https://lapse.hackclub.com/timelapse/abc", devlog.reload.lapse_url + assert_equal [ "https://lapse.hackclub.com/timelapse/abc" ], devlog.reload.lapse_urls end test "a normal resync moves a lapse_url into an orphaned link when its title disappears" do run_job("# Wiring\n\n**Total time spent: 30m**\n", commit_date: @today) devlog = Devlog.find_by(project: @project, title: "Wiring") - devlog.update!(lapse_url: "https://lapse.hackclub.com/timelapse/abc") + devlog.update!(lapse_urls: [ "https://lapse.hackclub.com/timelapse/abc" ]) run_job("# Soldering\n\n**Total time spent: 1h**\n", commit_date: @today) - assert_nil devlog.reload.lapse_url + assert_empty devlog.reload.lapse_urls link = @project.orphaned_lapse_links.find_by(title: "Wiring") assert_equal "https://lapse.hackclub.com/timelapse/abc", link.lapse_url end test "a clear resync restores a lapse_url when the title still exists" do run_job("# Wiring\n\n**Total time spent: 30m**\n", commit_date: @today) - Devlog.find_by(project: @project, title: "Wiring").update!(lapse_url: "https://lapse.hackclub.com/timelapse/abc") + Devlog.find_by(project: @project, title: "Wiring").update!(lapse_urls: [ "https://lapse.hackclub.com/timelapse/abc" ]) run_job("# Wiring\n\n**Total time spent: 2h**\n", commit_date: @today, clear: true) devlog = Devlog.find_by(project: @project, title: "Wiring") - assert_equal "https://lapse.hackclub.com/timelapse/abc", devlog.lapse_url + assert_equal [ "https://lapse.hackclub.com/timelapse/abc" ], devlog.lapse_urls assert_equal 0, @project.orphaned_lapse_links.count end test "a clear resync preserves a vanished title's lapse_url as an orphaned link instead of losing it" do run_job("# Wiring\n\n**Total time spent: 30m**\n", commit_date: @today) - Devlog.find_by(project: @project, title: "Wiring").update!(lapse_url: "https://lapse.hackclub.com/timelapse/abc") + Devlog.find_by(project: @project, title: "Wiring").update!(lapse_urls: [ "https://lapse.hackclub.com/timelapse/abc" ]) run_job("# Soldering\n\n**Total time spent: 1h**\n", commit_date: @today, clear: true) diff --git a/test/models/project_test.rb b/test/models/project_test.rb index 32b27e91..d3b7c429 100644 --- a/test/models/project_test.rb +++ b/test/models/project_test.rb @@ -68,7 +68,7 @@ # index_projects_on_discarded_at (discarded_at) # index_projects_on_flagged_by_id (flagged_by_id) # index_projects_on_flagged_for_review_at (flagged_for_review_at) -# index_projects_on_linked_project_id_for_build_reviews (linked_project_id) UNIQUE WHERE (build_review = true) +# index_projects_on_linked_project_id_for_build_reviews (linked_project_id) UNIQUE WHERE ((build_review = true) AND (discarded_at IS NULL)) # index_projects_on_macondo_project_id (macondo_project_id) UNIQUE # index_projects_on_parent_project_id (parent_project_id) # index_projects_on_requirements_checked_at (requirements_checked_at) diff --git a/test/services/journal_digest_test.rb b/test/services/journal_digest_test.rb index 2a9819e5..21b965e8 100644 --- a/test/services/journal_digest_test.rb +++ b/test/services/journal_digest_test.rb @@ -6,10 +6,10 @@ class JournalDigestTest < ActiveSupport::TestCase @user = @project.user end - def entry(entry_date:, hours:, content: "x" * 300, created_at: nil, lapse_url: nil, title: "Entry") + def entry(entry_date:, hours:, content: "x" * 300, created_at: nil, lapse_urls: [], title: "Entry") @project.devlogs.create!( user: @user, title: title, content: content, time_hours: hours, - entry_date: entry_date, lapse_url: lapse_url, + entry_date: entry_date, lapse_urls: lapse_urls, created_at: created_at || entry_date.to_time ) end @@ -72,7 +72,7 @@ def entry(entry_date:, hours:, content: "x" * 300, created_at: nil, lapse_url: n test "counts images and timelapse links" do entry(entry_date: Date.new(2026, 2, 3), hours: 1.0, content: "progress ![shot](a.png) #{'x' * 200}") - entry(entry_date: Date.new(2026, 2, 4), hours: 1.0, lapse_url: "https://example.com/lapse") + entry(entry_date: Date.new(2026, 2, 4), hours: 1.0, lapse_urls: [ "https://example.com/lapse" ]) digest = JournalDigest.build(@project) assert_equal 1, digest["with_images"]