OpenFn / lightning

OpenFn/Lightning ⚡️ is the newest version of the OpenFn DPG and provides a web UI to visually manage complex workflow automation projects.
https://openfn.github.io/lightning/
GNU Lesser General Public License v3.0
127 stars 34 forks source link

Github integration not working on demo after nightly build #1058

Closed taylordowns2000 closed 1 year ago

taylordowns2000 commented 1 year ago

Not 100% sure that this is the problem, but on when we reset the demo, we should probably:

  1. Destroy the existing project_repo_connection
  2. Set up a new project_repo_connection using the installation ID for https://github.com/OpenFn/demo-openhie.

See tear_down and setup_demo in Lightning.Demo for a starting point

taylordowns2000 commented 1 year ago

hey @zacck , when you recreate this connection by hand, are you doing it with the superuser account or the demo@openfn.org account? wondering if that's why it's not working. (we'll need to consider this as we build in the functionality suggested above to rebuild this connection each night.)

zacck commented 1 year ago

@taylordowns2000 I reckon I used both, but the most recent one has been from demo@openfn.org

taylordowns2000 commented 1 year ago

ok. @zacck , I'm seeing this error when attempting to delete projects:

** (Postgrex.Error) ERROR 23503 (foreign_key_violation) update or delete on table "projects" violates foreign key constraint "project_repos_project_id_fkey" on table "project_repos"

    table: project_repos
    constraint: project_repos_project_id_fkey

Key (id)=(4adf2644-ed4e-4f97-a24c-ab35b3cb1efa) is still referenced from table "project_repos".

the behaviour we want is that the project_repo should be deleted (by cascade) when a project is deleted. is it possible that this is reversed? or there's something else that needed to be added to the migration here?

defmodule Lightning.Repo.Migrations.CreateRepoProjectConnection do
  use Ecto.Migration

  def change do
    create table(:project_repos, primary_key: false) do
      add :id, :binary_id, primary_key: true
      add :github_installation_id, :string
      add :repo, :string
      add :branch, :string
      add :project_id, references(:projects, type: :binary_id, on_delete: :delete_all)
      add :user_id, references(:users, type: :binary_id, on_delete: :nilify_all)

      timestamps()
    end
  end
end
zacck commented 1 year ago

@taylordowns2000 I have been looking at this and I am confused, the referencing row should have been deleted. I wonder if I ammreading the docs wrong Ill make some time shortly to run some experiments then see if I can get a solution to this.

zacck commented 1 year ago

@taylordowns2000 my code is wrong the direction of this directive is the other way round, The solution would be to add the relation to projects and handle the :on_delete from that end. Is this amiable with you?

taylordowns2000 commented 1 year ago

ahh ok. let me get my bearings here:

  1. the current code would cascade delete projects when a project_repo is deleted?
  2. but you'll change that so that associated project_repos will be cascade deleted when a project is deleted?

if that's the current status then yes, please do make that change. the desired behaviour is that if a project is deleted, any associated project_repo records will also be deleted.

taylordowns2000 commented 1 year ago

After more explanation here @zacck , I think your suspicion about changes to migrations and when the original migrations were run is spot on. After dropping the DB entirely, the cascades seem to work as expected.

If this matches your expectations for the way these lines* of the current migration work then I think the only thing remaining on this ticket is restoring the project_repo after the sync. i'll take that on so you can focus on wrapping up docs and the two remaining tickets: #1046 and #1059

thoughts?

*these lines:

      add :project_id, references(:projects, type: :binary_id, on_delete: :delete_all)
      add :user_id, references(:users, type: :binary_id, on_delete: :nilify_all)