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

Define item type: Task or Note #14

Open
3 tasks done
SimonLab opened this issue Apr 16, 2020 · 6 comments
Open
3 tasks done

Define item type: Task or Note #14

SimonLab opened this issue Apr 16, 2020 · 6 comments
Assignees
Labels
awaiting-review enhancement New feature or request

Comments

@SimonLab
Copy link
Member

SimonLab commented Apr 16, 2020

ref: dwyl/app#266 (comment)
An item can be either a task or a note.

  • Create a new migration to add a boolean task field (default true)
  • Update changeset to cast the field task
  • Update captures tests to add and test the task field
@SimonLab SimonLab added the enhancement New feature or request label Apr 16, 2020
@SimonLab SimonLab self-assigned this Apr 16, 2020
SimonLab added a commit that referenced this issue Apr 16, 2020
SimonLab added a commit that referenced this issue Apr 16, 2020
SimonLab added a commit that referenced this issue Apr 17, 2020
SimonLab added a commit that referenced this issue Apr 20, 2020
@SimonLab
Copy link
Member Author

To create the many_to_many association between captures and tags I had to remember how to do this with Ecto:

@SimonLab
Copy link
Member Author

SimonLab commented Apr 20, 2020

The tags table doesn't contains any items at the moment.
We can create a seed files to add the task and note tags.
We also need to add the /api/tags endpoint to get all the tags created by the authenticated user. This will allow the user to see which tags can be added to a capture or to create new ones if they don't exist yet.
This endpoint should also return the default tags, ie the ones where the id_person field is null (created by the seed)

SimonLab added a commit that referenced this issue Apr 20, 2020
SimonLab added a commit that referenced this issue Apr 20, 2020
@SimonLab
Copy link
Member Author

At the moment the coverage is not great: #16 (comment)

@SimonLab
Copy link
Member Author

While updating the captures test the following error occurs:

** (Postgrex.Error) ERROR 23502 (not_null_violation) null value in column "inserted_at" violates not-null constraint

The following comments explain that with Repo.insert_all the timestamps() function used in the schema is not used: https://elixirforum.com/t/not-null-violation-on-seeding-my-database-timestamps-macro-failing/19858

I'm using insert_all to insert all the tags of a capture:

defp insert_and_get_all(tags, id_person) do
maps = Enum.map(tags, &%{text: &1, id_person: id_person})
Repo.insert_all(Tag, maps, on_conflict: :nothing)
Repo.all(from t in Tag, where: t.name in ^tags)
end

We can update manually the timestamp fields with:

data =
  data
    |> Enum.map(fn(row) ->
        row
          |> Map.put(:inserted_at, Timex.now)
          |> Map.put(:updated_at, Timex.now)
      end)

Repo.insert_all(Table, data)

see also: elixir-ecto/ecto#1932

SimonLab added a commit that referenced this issue Apr 21, 2020
SimonLab added a commit that referenced this issue Apr 21, 2020
@SimonLab
Copy link
Member Author

The context files are now fully tested.
Most of the remaining files to test are the controllers and the view.
Because the controller is using the view I think only testing the controllers should also cover the views.
The main difficulty is to create a valid jwt to be able to test the private endpoints/controller.

SimonLab added a commit that referenced this issue Apr 22, 2020
@SimonLab
Copy link
Member Author

I've tested different way to be able to test the controllers that requires a jwt in the request headers and the easier one at the end is to update the plug authentication function to add a condition depending on the running environment:

  def call(conn, _) do
    if Mix.env() == :test do
      assign(conn, :person, %{email: "email", name: "name", id_person: 42})
    else
      case AppApiWeb.AuthServiceApi.get_person_information(conn) do
        {:error, _} -> unauthorized(conn)
        {:ok, person} -> assign(conn, :person, person)
      end
    end
  end

This way we avoid calling the external authentication system and we bypass all the Google/Github Oauth request flow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant