Skip to content

Fix seeding #958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
paulcretu wants to merge 4 commits into
base: main
Choose a base branch
from
+19 −4
Open

Fix seeding #958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
91142b5
Add logging to feed seeding
paulcretu Sep 21, 2025
c5a535c
Enable errors when seeding fails
paulcretu Sep 21, 2025
a92c84d
Fix broken seeding due to maintainer emails
paulcretu Sep 3, 2025
c6abb09
Default auto seeding to enabled in frontend dev env
paulcretu Aug 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions server/lib/orcasite/radio/feed.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,14 @@ defmodule Orcasite.Radio.Feed do
description "A comma-separated string of longitude and latitude"
end

argument :maintainer_emails, {:array, :string}, default: []

@skanderm skanderm Sep 21, 2025

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.

If the argument is an attribute and you're setting the attribute to the argument, you can simply add it to the accept list above and skip the argument and change _attribute changeset. You could add it to the upsert_fields too, but it's less important since this is just for seeding.

@paulcretu paulcretu Sep 21, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm I need it to default to empty list though, in case the API returns nil for maintainer_emails (which it will because it's an auth only field). So I guess I could remove the argument but I still need the change no? Do you have a better suggestion? Curious how you would tackle it, I don't really know what's idiomatic

@skanderm skanderm Sep 21, 2025

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.

Ok yeah really good point. In prod we don't want that to be nil, but an empty list works.


change &change_lat_lng/2

change fn changeset, _context ->
maintainer_emails = Ash.Changeset.get_argument(changeset, :maintainer_emails) || []
Ash.Changeset.change_attribute(changeset, :maintainer_emails, maintainer_emails)
end
Comment on lines +264 to +271

@coderabbitai coderabbitai Bot Sep 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

🧩 Analysis chain

Ensure upserts can update maintainer_emails; also normalize values.

Currently not included in upsert_fields, so existing rows won’t update on conflict; also normalize to lower-case/unique.

Apply both diffs:

  • Normalize on create:
-  change fn changeset, _context ->
-    maintainer_emails = Ash.Changeset.get_argument(changeset, :maintainer_emails) || []
-    Ash.Changeset.change_attribute(changeset, :maintainer_emails, maintainer_emails)
-  end
+  change fn changeset, _context ->
+    maintainer_emails =
+      (Ash.Changeset.get_argument(changeset, :maintainer_emails) || [])
+      |> Enum.map(&String.downcase/1)
+      |> Enum.uniq()
+
+    Ash.Changeset.change_attribute(changeset, :maintainer_emails, maintainer_emails)
+  end
  • Allow updates on conflict (outside this hunk; in upsert_fields):
       upsert_fields [
         :name,
         :node_name,
         :intro_html,
         :image_url,
         :visible,
         :bucket,
         :bucket_region,
         :cloudfront_url,
         :dataplicity_id,
         :orcahello_id,
-        :location_point
+        :location_point,
+        :maintainer_emails
       ]

🏁 Script executed:

#!/bin/bash
# Verify DB migration exists for the new column
rg -n "maintainer_emails" -g "priv/**/migrations/*.exs" -C2 || echo "MIGRATION NOT FOUND"
# Verify GraphQL schema exposure matches policy (nullable, field policy)
rg -n "nullable_fields.*maintainer_emails" server/lib/orcasite/radio/feed.ex -C2

Length of output: 273

Add missing DB migration and update upsert logic

  • Create a new Ecto migration under priv/repo/migrations to add :maintainer_emails (type {:array, :string}, default []) to the feeds table.
  • In server/lib/orcasite/radio/feed.ex – inside the upsert_fields list, append :maintainer_emails; in the change hook, normalize with Enum.map(&String.downcase/1) and Enum.uniq().

GraphQL exposure (nullable_fields [:maintainer_emails]) is already in place.

🤖 Prompt for AI Agents 
In server/lib/orcasite/radio/feed.ex around lines 264 to 271, the PR missed
adding a DB migration and updating upsert logic for the new maintainer_emails
field: add an Ecto migration file under priv/repo/migrations that alters the
feeds table to add a maintainer_emails column of type {:array, :string} with
default [] and null: false (or appropriate nullability), and run mix
ecto.migrate; then update server/lib/orcasite/radio/feed.ex by appending
:maintainer_emails to the upsert_fields list and modify the change hook to
normalize the incoming maintainer_emails via Enum.map(&String.downcase/1) |>
Enum.uniq() before calling Ash.Changeset.change_attribute (keeping
change_lat_lng/2 intact). Ensure the migration and schema/upsert changes remain
consistent and tests/GraphQL nullable exposure still pass.

Comment on lines +268 to +271

@skanderm skanderm Sep 21, 2025

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.

This can still be simplified to:

Suggested change
change fn changeset, _context ->
maintainer_emails = Ash.Changeset.get_argument(changeset, :maintainer_emails) || []
Ash.Changeset.change_attribute(changeset, :maintainer_emails, maintainer_emails)
end
change set_attribute(:maintainer_emails, arg(:maintainer_emails), set_when_nil?: false)

See: https://hexdocs.pm/ash/Ash.Resource.Change.Builtins.html#set_attribute/3

end

update :update do
Expand Down
4 changes: 2 additions & 2 deletions server/lib/orcasite/radio/seed.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ defmodule Orcasite.Radio.Seed do
default: fn -> DateTime.utc_now() |> DateTime.add(-2, :minute) end

run fn %{arguments: %{start_time: start_time, end_time: end_time}}, _ ->
__MODULE__.feeds()
__MODULE__.feeds!()

feeds = Orcasite.Radio.Feed |> Ash.read!()

Expand Down Expand Up @@ -90,7 +90,7 @@ defmodule Orcasite.Radio.Seed do
argument :limit, :integer, allow_nil?: false, default: 100

run fn %{arguments: %{limit: limit}}, _ ->
__MODULE__.feeds()
__MODULE__.feeds!()

feeds = Orcasite.Radio.Feed |> Ash.read!()

Expand Down
6 changes: 6 additions & 0 deletions server/lib/orcasite/radio/seed/changes/seed_feeds.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ defmodule Orcasite.Radio.Seed.Changes.SeedFeeds do

alias Orcasite.Radio.Seed.Utils

require Logger

@impl true
def change(changeset, _opts, _context) do
changeset
Expand All @@ -17,10 +19,14 @@ defmodule Orcasite.Radio.Seed.Changes.SeedFeeds do
|> Ash.bulk_create(Orcasite.Radio.Feed, :create, return_errors?: true, authorize?: false)
|> case do
%{status: :success} ->
Logger.info("Successfully seeded #{count} feeds")

change
|> Ash.Changeset.force_change_attribute(:seeded_count, count)

%{errors: errors} ->
Logger.error("Error while seeding feeds: #{inspect(errors)}")

change
|> Ash.Changeset.add_error(errors)
end
Expand Down
6 changes: 4 additions & 2 deletions ui/src/pages/seed.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,10 @@ function toLocalISOString(date: Date) {
SeedPage.getLayout = getSimpleLayout;

export async function getStaticProps() {
const enableSeedFromProd = process.env.ENABLE_SEED_FROM_PROD === "true";
// Hide the seed page when `ENABLE_SEED_FROM_PROD` isn't enabled
const enableSeedFromProd = process.env.ENABLE_SEED_FROM_PROD
? process.env.ENABLE_SEED_FROM_PROD === "true"
: process.env.NODE_ENV === "development"; // default to enabled in dev

return !enableSeedFromProd ? { notFound: true } : { props: {} };
}

Expand Down
Loading