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

Quest roles BE #1075

Merged
merged 13 commits into from
Feb 2, 2022
Merged

Quest roles BE #1075

merged 13 commits into from
Feb 2, 2022

Conversation

vidvidvid
Copy link
Collaborator

@vidvidvid vidvidvid commented Jan 24, 2022

Overview

What features/fixes does this PR include?

Added quest_roles needed for #1034
Added basic flag to determine which roles to display during onboarding and which to display when selecting roles for quests.

(Excuse me for multiple migrations, some of which were unnecessary)

paired-with: @dan13ram

@vidvidvid vidvidvid requested a review from dan13ram January 24, 2022 18:25
@vidvidvid vidvidvid self-assigned this Jan 24, 2022
@vercel
Copy link

vercel bot commented Jan 24, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/metafam/the-game/6SRTFBh6rQuiPbMHRY49BbXKjJd3
✅ Preview: https://the-game-git-quest-roles-demo-1-metafam.vercel.app

@github-actions
Copy link

Successfully deployed a Preview of this Pull Request
Frontend
Hasura

@vidvidvid
Copy link
Collaborator Author

vidvidvid commented Jan 24, 2022

@dan13ram please confirm if i included everything i should have included in regards to BE for #1034 in this pr :)

@dan13ram dan13ram requested a review from alalonde January 25, 2022 15:41
Copy link
Contributor

@alalonde alalonde left a comment

Choose a reason for hiding this comment

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

can you consolidate the migrations? From what I can see there should only be 3. You can test them locally via hasura migrate apply ...

@vidvidvid
Copy link
Collaborator Author

rebasing this branch onto develop and running yarn update-schema completely messes up the schema.graphql. any idea why is that happening? @dysbulic @dan13ram @alalonde ?

@dysbulic
Copy link
Member

rebasing this branch onto develop and running yarn update-schema completely messes up the schema.graphql.

@vidvidvid, have you run yarn && yarn docker:clean && docker-compose up --build to rebuild the database? Your existing db might have migrations in it that are out of sync given the large number of migrations in #943.

When you say it messes up schema.graphql, what do you mean? What sorts of errors are you getting?

@vidvidvid
Copy link
Collaborator Author

@vidvidvid, have you run yarn && yarn docker:clean && docker-compose up --build to rebuild the database? Your existing db might have migrations in it that are out of sync given the large number of migrations in #943.

Not yet, I'll do that and let you know how it works.

When you say it messes up schema.graphql, what do you mean? What sorts of errors are you getting?

It builds a completely different structure, as if it didn't take into the consideration all of your migrations.

@vidvidvid
Copy link
Collaborator Author

vidvidvid commented Jan 26, 2022

@dysbulic yarn && yarn docker:clean && docker-compose up --build fails at yarn:

vidtopolovec@Vids-MBP TheGame % yarn
yarn install v1.19.0
[1/5] 🔍  Validating package.json...
[2/5] 🔍  Resolving packages...
warning Resolution field "typescript@4.5.4" is incompatible with requested version "typescript@^3.7.3"
warning Resolution field "typescript@4.5.4" is incompatible with requested version "typescript@^2.6.2"
warning Resolution field "typescript@4.5.4" is incompatible with requested version "typescript@^2.6.2"
success Already up-to-date.
$ lerna run prepare && husky install
lerna notice cli v4.0.0
lerna info versioning independent
lerna info Executing command in 1 package: "yarn run prepare"
lerna ERR! yarn run prepare exited 1 in '@metafam/web'
lerna ERR! yarn run prepare stdout:
yarn run v1.19.0
$ yarn generate
$ graphql-codegen --config=codegen.yml
[11:25:29] Parse configuration [started]
[11:25:30] Parse configuration [completed]
[11:25:30] Generate outputs [started]
[11:25:30] Generate ./graphql/autogen/types.ts [started]
[11:25:30] Load GraphQL schemas [started]
[11:25:30] Load GraphQL schemas [failed]
[11:25:30] → Failed to load schema
[11:25:30] Generate ./graphql/autogen/types.ts [failed]
[11:25:30] → Failed to load schema
[11:25:30] Generate outputs [failed]
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
lerna ERR! yarn run prepare stderr:
Something went wrong
error Command failed with exit code 1.
error Command failed with exit code 1.
lerna ERR! yarn run prepare exited 1 in '@metafam/web'
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

Does yarn generate fail because the schema.graphql is not valid?

@dysbulic
Copy link
Member

Does yarn generate fail because the schema.graphql is not valid?

It is possible… Try running yarn generate. I added a DEBUG=1 environment variable to that command that should cause it to print the file and line where it is failing.

@vidvidvid
Copy link
Collaborator Author

@dysbulic I had a missing query in the schema, yarn generate worked after i added it 👍

@vidvidvid
Copy link
Collaborator Author

can you consolidate the migrations? From what I can see there should only be 3. You can test them locally via hasura migrate apply ...

Will look into it 👍

@github-actions
Copy link

Successfully deployed a preview of this pull request:

@vidvidvid
Copy link
Collaborator Author

@alalonde I consolidated the migrations, but I'm not sure how to merge this and that, could you help me out with that or could we perhaps just push this?

@github-actions
Copy link

Successfully deployed a preview of this pull request:

@vidvidvid vidvidvid changed the title Quest roles Quest roles BE Jan 30, 2022
@vidvidvid vidvidvid added quests ready-for-review Add this label to your PR when its ready for review labels Jan 30, 2022
@github-actions
Copy link

Successfully deployed a preview of this pull request:

Copy link
Contributor

@alalonde alalonde left a comment

Choose a reason for hiding this comment

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

You can have multiple lines in a given migration, both up and down

@@ -73,6 +73,7 @@ input CreateQuestInput {
externalLink : String
repetition : QuestRepetition_ActionEnum
cooldown : Int
roles_id : [uuid]!
Copy link
Contributor

Choose a reason for hiding this comment

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

let's make this camelcase for consistency

Copy link
Contributor

Choose a reason for hiding this comment

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

roleIds ?

@vidvidvid
Copy link
Collaborator Author

@alalonde I changed skills_id -> skillIds and rolesId -> roleIds, tested creating and editing quests in the client PR and everything works fine! :)

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Successfully deployed a preview of this pull request:

@vidvidvid
Copy link
Collaborator Author

You can have multiple lines in a given migration, both up and down

@alalonde so should i join the quest role migrations?
hasura/migrations/1641640073308_create_table_public_quest_role/up.sql:

CREATE TABLE "public"."quest_role"("quest_id" UUID NOT NULL, "role" text NOT NULL, "rank" integer, PRIMARY KEY ("quest_id","role") , FOREIGN KEY ("quest_id") REFERENCES "public"."quest"("id") ON UPDATE restrict ON DELETE cascade, FOREIGN KEY ("role") REFERENCES "public"."PlayerRole"("role") ON UPDATE restrict ON DELETE restrict, UNIQUE ("quest_id"));

and hasura/migrations/1642342313101_alter_table_public_quest_role_drop_constraint_quest_role_quest_id_key/up.sql:

alter table "public"."quest_role" drop constraint "quest_role_quest_id_key";
by putting one below the other in the same file?

@vidvidvid vidvidvid requested a review from alalonde February 2, 2022 14:51
@alalonde
Copy link
Contributor

alalonde commented Feb 2, 2022

You can have multiple lines in a given migration, both up and down

@alalonde so should i join the quest role migrations? hasura/migrations/1641640073308_create_table_public_quest_role/up.sql:

CREATE TABLE "public"."quest_role"("quest_id" UUID NOT NULL, "role" text NOT NULL, "rank" integer, PRIMARY KEY ("quest_id","role") , FOREIGN KEY ("quest_id") REFERENCES "public"."quest"("id") ON UPDATE restrict ON DELETE cascade, FOREIGN KEY ("role") REFERENCES "public"."PlayerRole"("role") ON UPDATE restrict ON DELETE restrict, UNIQUE ("quest_id"));

and hasura/migrations/1642342313101_alter_table_public_quest_role_drop_constraint_quest_role_quest_id_key/up.sql:

alter table "public"."quest_role" drop constraint "quest_role_quest_id_key"; by putting one below the other in the same file?

Just an FYI, no need this time :)

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Successfully deployed a preview of this pull request:

@alalonde alalonde merged commit 5b61955 into develop Feb 2, 2022
@alalonde alalonde deleted the quest-roles-demo-1 branch February 2, 2022 16:54
@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Successfully undeployed the Preview of this Pull Request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quests ready-for-review Add this label to your PR when its ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants