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

Update base fields to have org scope #1314

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Update base fields to have org scope #1314

merged 1 commit into from
Dec 3, 2024

Conversation

bickelj
Copy link
Contributor

@bickelj bickelj commented Nov 7, 2024

Without this change the "gold" data feature does not return gold data. This is because the base fields need 'organization' scope. This changes the relevant base fields in one fell swoop.

Issue #1138

@bickelj bickelj requested a review from slifty November 7, 2024 22:09
@bickelj
Copy link
Contributor Author

bickelj commented Nov 7, 2024

@slifty As I pushed this I thought maybe you will want this change to be done via a GET followed by a PUT in the API.

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.34%. Comparing base (d7c27fd) to head (713e24a).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1314   +/-   ##
=======================================
  Coverage   87.34%   87.34%           
=======================================
  Files         185      185           
  Lines        2387     2387           
  Branches      321      314    -7     
=======================================
  Hits         2085     2085           
- Misses        276      302   +26     
+ Partials       26        0   -26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@slifty
Copy link
Member

slifty commented Nov 10, 2024

To your point above @bickelj I don't think that a migration is the right way to adjust seed data or production data.

I think we should update the seed sql in this PR instead and then for any instances just run some scripted curls to make these changes via the existing API instance (any script to do that will be simple and IMHO doesn't need to be included in the repository, but could be part of the PR description instead)

@bickelj
Copy link
Contributor Author

bickelj commented Nov 22, 2024

@slifty I wouldn't call it simple, but it seems to do the trick:

API_DOMAIN='api-test.philanthropydatacommons.org'; TOKEN='yada.yada.yada'; curl -4 https://${API_DOMAIN}/baseFields \
  | jq -c '.[] | select(.shortCode | startswith("organization_")) | select(.scope == "proposal") | {id: .id, label: .label, description: .description, scope: "organization", dataType: .dataType, shortCode: .shortCode}' \
  | tr -d '{' \
  | sed 's/"id":\([0-9]*\),\(.*\)/--next -X '\''PUT'\'' -H '\''content-type: application\/json'\'' -H '\''Authorization: Bearer '${TOKEN}''\'' -d '\''{\2'\'' https:\/\/'${API_DOMAIN}'\/\1 /g' \
  | xargs -0 | tr '\n' ' '

Except that a real token is too long: xargs: argument line too long. So take the output,
then copy a chunk, prepend curl command, paste. Get new tokens as needed.

Then after pasting and running all that, run this to verify, this should come up empty because it filters on starting with "organization" and scope is "proposal" (all should be "organization" now):

API_DOMAIN='api-test.philanthropydatacommons.org'; curl -4 https://${API_DOMAIN}/baseFields \
  | jq -c '.[] | select(.shortCode | startswith("organization_")) | select(.scope == "proposal") | {id: .id, label: .label, description: .description, scope: "organization", dataType: .dataType, shortCode: .shortCode}' \
  | tr -d '{'

So that was an interesting exercise but I think running a SQL command would have been the better way.

@bickelj
Copy link
Contributor Author

bickelj commented Nov 22, 2024

I just ran the proposed seed on a fresh database from main at 2af1e4b and it succeeded.

Copy link
Member

@slifty slifty left a comment

Choose a reason for hiding this comment

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

w00t!

Also yes I stand corrected on the "simple" ;)

(this update to the seed looks good, and thanks for sharing the way to manually update on live data)

Without this change the "gold" data feature does not return gold data.
This is because the base fields need 'organization' scope. This changes
the relevant base fields in one fell swoop.

If you have no organization-scoped base fields in your database, run
this on your database:

```sql
UPDATE base_fields SET scope = 'organization'
WHERE short_code LIKE 'organization_%' AND scope = 'proposal';
```

Alternatively, if you wish to do the change as it happened in
production on 2024-11-22, you may use this script:
```bash
API_DOMAIN='api-test.philanthropydatacommons.org'; TOKEN='yada.yada.yada'; curl -4 https://${API_DOMAIN}/baseFields \
  | jq -c '.[] | select(.shortCode | startswith("organization_")) | select(.scope == "proposal") | {id: .id, label: .label, description: .description, scope: "organization", dataType: .dataType, shortCode: .shortCode}' \
  | tr -d '{' \
  | sed 's/"id":\([0-9]*\),\(.*\)/--next -X '\''PUT'\'' -H '\''content-type: application\/json'\'' -H '\''Authorization: Bearer '${TOKEN}''\'' -d '\''{\2'\'' https:\/\/'${API_DOMAIN}'\/\1 /g' \
  | xargs -0 | tr '\n' ' '
```

With a long JWT, you cannot use it to set `TOKEN` because its too long
for args to be sent to `curl`, so keep it as `yada.yada.yada` and do a
find and replace separately in a text file. Then use multiple `curl`
commands with pastes of args from the output of the above, updating
your JWT as needed when it expires. To check that the updates happened,
you can look at the base fields manually and/or use the following:

```bash
API_DOMAIN='api-test.philanthropydatacommons.org'; curl -4 https://${API_DOMAIN}/baseFields \
  | jq -c '.[] | select(.shortCode | startswith("organization_")) | select(.scope == "proposal") | {id: .id, label: .label, description: .description, scope: "organization", dataType: .dataType, shortCode: .shortCode}' \
  | tr -d '{'
```

This should come up blank if all your organization base fields were
successfully updated. Of course, set API_DOMAIN to your local domain
in the above commands.

Issue #1138
@bickelj bickelj merged commit 7eedebe into main Dec 3, 2024
6 checks passed
@bickelj bickelj deleted the org-scope-base-fields branch December 3, 2024 15:31
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