refactor: address review comments for join request settings
Some checks reported errors
continuous-integration/drone/push Build was killed
continuous-integration/drone/promote/production Build is passing

This commit is contained in:
Simon 2026-03-10 16:47:38 +01:00
parent 05e2a298fe
commit 21812542ad
Signed by: simon
GPG key ID: 40E7A58C4AA1EDB2
6 changed files with 29 additions and 15 deletions

View file

@ -1140,6 +1140,12 @@ let liveSocket = new LiveSocket("/live", Socket, {
})
```
**Vendor assets (third-party JS):**
Some JavaScript libraries are committed as vendored files in `assets/vendor/` (e.g. `topbar`, `sortable.js`) when they are not available as npm packages or we need a specific build. Document their origin and how to update them:
- **Sortable.js** (`assets/vendor/sortable.js`): From [SortableJS](https://github.com/SortableJS/Sortable), version noted in the file header (e.g. `/*! Sortable 1.15.6 - MIT ... */`). To update: download the desired release from the repo and replace the file; keep the header comment for traceability.
### 3.8 Code Quality: Credo
**Static Code Analysis:**

View file

@ -219,14 +219,21 @@ Hooks.SortableList = {
if (this.grabbedRowId == null) return
// Do not move into a locked row (e.g. email always first)
if (e.key === "ArrowUp" && idx > 0) {
e.preventDefault()
this.pushEvent(this.reorderEvent, { from_index: idx, to_index: idx - 1 })
announce(`Position ${idx} of ${total}.`)
const targetRow = rows[idx - 1]
if (!this.isLocked(targetRow)) {
e.preventDefault()
this.pushEvent(this.reorderEvent, { from_index: idx, to_index: idx - 1 })
announce(`Position ${idx} of ${total}.`)
}
} else if (e.key === "ArrowDown" && idx < total - 1) {
e.preventDefault()
this.pushEvent(this.reorderEvent, { from_index: idx, to_index: idx + 1 })
announce(`Position ${idx + 2} of ${total}.`)
const targetRow = rows[idx + 1]
if (!this.isLocked(targetRow)) {
e.preventDefault()
this.pushEvent(this.reorderEvent, { from_index: idx, to_index: idx + 1 })
announce(`Position ${idx + 2} of ${total}.`)
}
}
}

View file

@ -60,6 +60,10 @@ defmodule Mv.Membership.Setting do
domain: Mv.Membership,
data_layer: AshPostgres.DataLayer
# Used in join_form_field_ids validation (compile-time to avoid recompiling regex and list on every validation)
@uuid_pattern ~r/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i
@valid_join_form_member_fields Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)
postgres do
table "settings"
repo Mv.Repo
@ -255,16 +259,10 @@ defmodule Mv.Membership.Setting do
field_ids = Ash.Changeset.get_attribute(changeset, :join_form_field_ids)
if is_list(field_ids) and field_ids != [] do
valid_member_fields =
Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)
uuid_pattern =
~r/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i
invalid_ids =
Enum.reject(field_ids, fn id ->
is_binary(id) and
(id in valid_member_fields or Regex.match?(uuid_pattern, id))
(id in @valid_join_form_member_fields or Regex.match?(@uuid_pattern, id))
end)
if Enum.empty?(invalid_ids) do
@ -442,6 +440,7 @@ defmodule Mv.Membership.Setting do
attribute :join_form_field_ids, {:array, :string} do
allow_nil? true
default []
public? true
description "Ordered list of field IDs shown on the join form. Each entry is a member field name (e.g. 'email') or a custom field UUID. Email is always present after normalization."

View file

@ -54,7 +54,7 @@ defmodule Mv.Membership.Setting.Changes.NormalizeJoinFormSettings do
base = if is_map(required_config), do: required_config, else: %{}
base
|> Map.filter(fn {key, _} -> key in field_ids end)
|> Map.take(field_ids)
|> Map.put("email", true)
end
end

View file

@ -990,7 +990,7 @@ defmodule MvWeb.CoreComponents do
/>
</th>
<th :if={@action != []} class={table_th_sticky_class(@sticky_header)}>
{gettext("Actions")}
<span class="sr-only">{gettext("Actions")}</span>
</th>
</tr>
</thead>

View file

@ -11,6 +11,8 @@ defmodule Mv.Membership.SettingJoinFormTest do
- `Mv.Membership.get_join_form_allowlist/0` is implemented and returns the allowlist
for the public join form (subtask 4).
"""
# Settings is a singleton; tests mutate shared DB state. We use async: false and on_exit to restore
# original values because Ecto Sandbox transaction rollback does not apply to this singleton pattern.
use Mv.DataCase, async: false
alias Mv.Constants