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

[EdgeDB] Add Pins Seeding #3212

Merged
merged 1 commit into from
May 22, 2024
Merged

[EdgeDB] Add Pins Seeding #3212

merged 1 commit into from
May 22, 2024

Conversation

bryanjnelson
Copy link
Contributor

@bryanjnelson bryanjnelson commented May 16, 2024

Monday task

Description

Add seeding for pinnable types; some test users now have populated pins for projects, languages, and/or partners

Ready for review checklist

Use [N/A] if the item is not applicable to this PR or remove the item

  • Change the task url above to the actual Monday task
  • [N/A] Add/update tests if needed
  • Add reviewers to this PR

@CarsonF
Copy link
Member

CarsonF commented May 20, 2024

Yeah I'd have pins in its own file.

Update could simply be:

update User
filter .realFirstName = "Bilbo"
set {
  pins += (
    select Mixin::Pinnable
    filter [is Mixin::Named].name = {"Sindarin", "Quenya", "South Downs"}
  )
};

@bryanjnelson
Copy link
Contributor Author

bryanjnelson commented May 21, 2024

@CarsonF - Do you think there is enough benefit in keeping the json separated like this into languages, partners, and projects even though they could technically just be one big list of pinnables?

with
  usersJson := to_json('[
    {
      "user": "Bilbo",
      "pinnables": {
        "languages": ["Sindarin", "Quenya"],
        "partners": ["Dwarvish/Elvish Alliance", "Fellowship of Halfing Languages"],
        "projects": ["Emyn Muil", "Arnor Lake", "South Downs"]
      },
    }
  ]'),

Basically does seeing that structure at the top introduce enough benefit to see what type of pins we are adding, over flattening that structure into a list of names and having less code overall?

@CarsonF
Copy link
Member

CarsonF commented May 21, 2024

I don't see much benefit...I would just flatten

@bryanjnelson
Copy link
Contributor Author

bryanjnelson commented May 21, 2024

@CarsonF - This seems to modifying the data correctly, but the printed output is odd to me:

[
  {
    "Modified Users": [
      "Bilbo Baggins",
      "Bilbo Baggins",
      "Bilbo Baggins",
      "Peregrin Took",
      "Peregrin Took",
      "Peregrin Took",
      "Aragorn Son of Arathorn",
      "Aragorn Son of Arathorn",
      "Aragorn Son of Arathorn"
    ]
  }
]

I would expect this:

[
  {
    "Modified Users": [
      "Bilbo Baggins",
      "Peregrin Took",
      "Aragorn Son of Arathorn"
    ]
  }
]

Probably something simple?

@CarsonF
Copy link
Member

CarsonF commented May 21, 2024

Probably something simple?

Need a distinct somewhere.
Either

users := distinct (...)

or

modified := distinct (...)

or

modified := (select distinct users ...)

all work.

@bryanjnelson bryanjnelson changed the title Add Pins to User Seeding [EdgeDB] Add Pins Seeding May 22, 2024
@bryanjnelson bryanjnelson marked this pull request as ready for review May 22, 2024 16:28
@CarsonF CarsonF merged commit d226f4d into develop May 22, 2024
15 checks passed
@CarsonF CarsonF deleted the 0946-edgedb-pins-seeding branch May 22, 2024 16:51
adam-soltech pushed a commit that referenced this pull request May 29, 2024
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.

2 participants