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

chore: Move hasPlanningData value to team_integrations table #2776

Merged
merged 5 commits into from
Feb 13, 2024

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Feb 12, 2024

What does this PR do?

  • Creates new publicly queryable team_integrations.has_planning_data column
  • Populates column with data from from teams.settings-->"hasPlanningData"
  • Updates frontend to read from new location
  • Note: teams.settings-->"hasPlanningData" value not removed / tidied up, we can maybe address this when we just stop the team_settings column?

Data migration

On the Pizza, the following query can be made to get a list of all teams which have has_planing_data set to true -

SELECT slug FROM teams
JOIN team_integrations
ON teams.id = team_integrations.team_id
WHERE has_planning_data
ORDER BY slug ASC;

This gives the following list which matches the previous value of the hardcoded digitalLandOrganisations (plus the recently added tewkesbury, barking-and-dagenham & west-berkshire).

barking-and-dagenham
barnet
birmingham
buckinghamshire
camden
canterbury
doncaster
gloucester
lambeth
medway
newcastle
opensystemslab
southwark
st-albans
tewkesbury
west-berkshire

Testing

  • On the Pizza I can query via Planning Data for the above teams
  • On the Pizza I cannot query a team not on this list

@DafyddLlyr DafyddLlyr force-pushed the dp/hasPlanningData-column branch from 2abb1b6 to b48ab78 Compare February 12, 2024 11:31
Copy link

github-actions bot commented Feb 12, 2024

Removed vultr server and associated DNS entries

@DafyddLlyr DafyddLlyr force-pushed the dp/hasPlanningData-column branch from b48ab78 to 466413d Compare February 12, 2024 12:53
Copy link

github-actions bot commented Feb 12, 2024

🤖 Hasura Change Summary compared a subset of table metadata including permissions:

Updated Tables (1)

  • public.team_integrations permissions:

    insert select update delete
    api /
    public
    4 added column permissions
    insert select update
    api ➕ has_planning_data
    public ➕ has_planning_data
    ➕ id
    ➕ team_id

@DafyddLlyr DafyddLlyr force-pushed the dp/hasPlanningData-column branch from 466413d to a4c74c6 Compare February 12, 2024 13:11
Comment on lines +6 to +9
UPDATE team_integrations
SET has_planning_data = COALESCE((teams.settings->>'hasPlanningData')::boolean, false)
FROM teams
WHERE team_integrations.team_id = teams.id;
Copy link
Contributor Author

@DafyddLlyr DafyddLlyr Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've manually re-ran this on the Pizza as the data sync happens after the Hasura migrations have already ran 🌀

This won't be necessary on staging and production.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good shout! I was also slightly bamboozled when everything appeared to be false

@DafyddLlyr DafyddLlyr requested a review from a team February 12, 2024 16:24
@DafyddLlyr DafyddLlyr marked this pull request as ready for review February 12, 2024 16:24
@Mike-Heneghan
Copy link
Contributor

nit

I maybe just need to look at this with fresh eyes tomorrow although when I run the query on the pizza I get more records than your list:

  1. barking-and-dagenham
  2. barnet
  3. birmingham
  4. buckinghamshire
  5. camden
  6. canterbury
  7. doncaster
  8. gloucester
  9. lambeth
  10. medway
  11. newcastle
  12. opensystemslab
  13. southwark
  14. st-albans
  15. tewkesbury
  16. west-berkshire

I get the same but additionally:

gateshead & open-digital-planning

i.e.

  1. barking-and-dagenham
  2. barnet
  3. birmingham
  4. buckinghamshire
  5. camden
  6. canterbury
  7. doncaster
  8. gateshead
  9. gloucester
  10. lambeth
  11. medway
  12. newcastle
  13. open-digital-planning
  14. opensystemslab
  15. southwark
  16. st-albans
  17. tewkesbury
  18. west-berkshire

I'm not sure if it's a problem as that seems to match what we'd see on team.settings so I think it's fine.

Although, the difference triggered a wee warning bell.

I'll have a proper look tomorrow.

Copy link
Contributor

@Mike-Heneghan Mike-Heneghan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and working on the pizza as expected for me 👍

@DafyddLlyr DafyddLlyr merged commit c233afe into main Feb 13, 2024
12 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/hasPlanningData-column branch February 13, 2024 15:29
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