fun_with_flags icon indicating copy to clipboard operation
fun_with_flags copied to clipboard

Add timestamps migration

Open vtm9 opened this issue 9 months ago • 10 comments

Hey @tompave, I’ve created the first PR containing just migration timestamps. I love how Oban handles migrations with versioning: https://github.com/oban-bg/oban/blob/main/lib/oban/migration.ex Maybe we can implement a similar migrator here, what do you think?

vtm9 avatar Mar 20 '25 23:03 vtm9

Thank you for the work and the PR!

Can you please test this version of the code with and without the migration applied? That's necessary to write upgrade instructions. Ideally, this should be a non-breaking change if we want to ship it in the 1.x line.

tompave avatar Mar 21 '25 09:03 tompave

It would also be good to modify the current migration so that users can copy just one migration file when installing the package for the first time.

For example, look at:

  • How the NOT NULL constraints were added to both the base migration and an "upgrade" migration, and the instructions in the changelog: https://github.com/tompave/fun_with_flags/commit/d25e4f00023bbce5867ba5b29dc1a7ab3ed3ddfe
  • How that extra upgrade migration was later removed: https://github.com/tompave/fun_with_flags/pull/161

I love how Oban handles migrations with versioning Maybe we can implement a similar migrator here, what do you think?

Maybe in v2.0, but I think it should not be in this PR.

tompave avatar Mar 21 '25 10:03 tompave

make it non breaking might be hard. I'll try something. Thank you

vtm9 avatar Mar 26 '25 10:03 vtm9

Hey @tompave, I’ve tried multiple approaches, but I still can’t get a good result. We can use a compile-time flag to include timestamps in the schema during compilation, but at some point, we’ll need to introduce a breaking change anyway.

Why not release version 2.0 with the breaking changes? That seems like the simplest path forward. I can provide a migration guide.

Most likely, existing users will just add a migration with timestamps, and new users will include it during table creation.

vtm9 avatar Mar 29 '25 12:03 vtm9

Yes, I was thinking that adding this to v2.0 would be best. In that case it's ok if it's a breaking change.

Please leave the PR open for now. I'll tag it with the v2.0 label.

tompave avatar Mar 30 '25 21:03 tompave

Just to make it easier to resume work for this later:

  • This was attempted in https://github.com/tompave/fun_with_flags/pull/154.
    • Plus a small fix in https://github.com/tompave/fun_with_flags/pull/159.
  • Some of my notes in 69ea3a8, on the gotchas of adding columns with a default value in production.
  • Later reverted in https://github.com/tompave/fun_with_flags/pull/160.

tompave avatar Apr 02 '25 09:04 tompave

hey @tompave , are there any blockers to release v 2.0? I also want to add timestamps to gate and flag, to show it in the FWF_UI, what do you think?

defmodule FunWithFlags.Flag do

  defstruct name: nil, gates: [], inserted_at: nil, updated_at: nil

defmodule FunWithFlags.Gate do


  defstruct [:type, :for, :enabled, :inserted_at, :updated_at]

  defmodule FunWithFlags.Store.Serializer.Ecto do
    @moduledoc false

    alias FunWithFlags.Flag
    alias FunWithFlags.Gate
    alias FunWithFlags.Store.Persistent.Ecto.Record

    @spec deserialize_flag(atom() | String.t(), list()) :: Flag.t()
    def deserialize_flag(name, []), do: Flag.new(to_atom(name), [])

    def deserialize_flag(name, list) when is_list(list) do
      gates =
        list
        |> Enum.sort_by(& &1.gate_type)
        |> Enum.map(&deserialize_gate(to_string(name), &1))
        |> Enum.reject(&(!&1))

      inserted_at = gates |> Enum.map(& &1.inserted_at) |> Enum.min()
      updated_at = gates |> Enum.map(& &1.updated_at) |> Enum.max()

      Flag.new(to_atom(name), gates, inserted_at, updated_at)
    end

vtm9 avatar Apr 02 '25 10:04 vtm9

hey @tompave , sorry for disturbing. I really want to merge timestamps 😄

vtm9 avatar Apr 08 '25 13:04 vtm9

Hey, because of personal reasons it's unlikely I'll be able to pay much attention to this in the short term. If timestamps are critical for you please use your fork. If you want, you can work against the v2 branch to make integration easier later.

tompave avatar Apr 10 '25 13:04 tompave

Hey! I completely understand that personal matters must come first. If there’s anything I can do to help test or collaborate on the v2 branch so that integration is smoother down the road, please let me know. Wishing you all the best, and thanks again for your time and support!

vtm9 avatar Apr 10 '25 21:04 vtm9