Skip to content
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.

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

Deduplicate data passed to FormPayload #137

Merged
merged 1 commit into from
Oct 9, 2024
Merged

Deduplicate data passed to FormPayload #137

merged 1 commit into from
Oct 9, 2024

Conversation

germsvel
Copy link
Owner

@germsvel germsvel commented Oct 9, 2024

Closes #110

What changed?

Currently, FormData keeps the data passed as a list of tuples. That's not an optimal strategy since it doesn't deal with duplicates well.

That being said, there are two competing behaviors that we need to handle:

  • One the one hand, we want to keep duplicates of list-like names (e.g. {"contact[]", "email"}.
  • On the other hand, we don't really want to keep duplicates of the same operation done multiple times (e.g. [{"email", "[email protected]"}, {"email", "[email protected]"}]).

In the latter case, we can imagine a user doing a fill_in twice, where the email field changed twice. In the first case, we might have something like a checkbox where the user can check multiple values.

Like we said, a list of tuples doesn't seem to be the optimal data structure for FormData. But while we have it, we handle deduplication of keys in FormPayload.new/1.

But we have to consider one more case. If a user calls:

session
|> fill_in("Email", with: "[email protected]")
|> fill_in("Email", with: "[email protected]")
|> fill_in("Email", with: "[email protected]")

The final record should show [email protected] as the email. If we do a simple Enum.uniq/1, we dedup [email protected], leaving [email protected] as the last operation -- which is not what we want. Thus, our current implementation has to reverse the list of data, unique it, and then reverse it again.

What changed?
=============

Currently, `FormData` keeps the data passed as a list of tuples. That's
not an optimal strategy since it doesn't deal with duplicates well.

That being said, there are two competing behaviors that we need to
handle:

- One the one hand, we want to keep duplicates of list-like `name`s
  (e.g. `{"contact[]", "email"}`.
- On the other hand, we don't really want to keep duplicates of the same
  operation done multiple times (e.g. `[{"email", "[email protected]"},
  {"email", "[email protected]"}]`).

In the latter case, we can imagine a user doing a `fill_in` twice, where
the email field changed twice. In the first case, we might have
something like a checkbox where the user can check multiple values.

Like we said, a list of tuples doesn't seem to be the optimal data
structure for `FormData`. But while we have it, we handle deduplication
of keys in `FormPayload.new/1`.

But we have to consider one more case. If a user calls:

```elixir
session
|> fill_in("Email", with: "[email protected]")
|> fill_in("Email", with: "[email protected]")
|> fill_in("Email", with: "[email protected]")
```

The final record should show `[email protected]` as the `email`. If we
do a simple `Enum.uniq/1`, we dedup `[email protected]`, leaving
`[email protected]` as the last operation -- which is not what we
want. Thus, our current implementation has to reverse the list of data,
unique it, and then reverse it again.
@germsvel germsvel merged commit e950ae4 into main Oct 9, 2024
3 checks passed
@germsvel germsvel deleted the dedup-list-data branch October 9, 2024 13:18
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.

1 participant