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

Agl/add is language of consultiung boolean #2983

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

adam-soltech
Copy link
Contributor

@adam-soltech adam-soltech commented Dec 6, 2023

ticket

┆Issue is synchronized with this Monday item by Unito

Copy link

github-actions bot commented Dec 6, 2023

🗞 GraphQL Summary

View schema changes
@@ -624,8 +624,11 @@
   displayNamePronunciation: String
   ethnologue: CreateEthnologueLanguage = {}
   hasExternalFirstScripture: Boolean = false
   isDialect: Boolean = false
+  isLanguageOfConsulting: Boolean = false
+  isLanguageOfReporting: Boolean
+  isLanguageOfWiderCommunication: Boolean
   isSignLanguage: Boolean = false
   leastOfThese: Boolean = false
   leastOfTheseReason: String
   name: String!
@@ -751,8 +754,9 @@
   countries: [ID!] = []
   fieldRegions: [ID!] = []
   financialReportingTypes: [FinancialReportingType!] = []
   globalInnovationsClient: Boolean
+  languageOfReportingId: ID
   languageOfWiderCommunicationId: ID
   languagesOfConsulting: [ID!] = []
   organizationId: ID!
   pmcEntityCode: String
@@ -2297,8 +2301,11 @@
   id: ID!
 
   """Whether this language is a dialect."""
   isDialect: SecuredBoolean!
+  isLanguageOfConsulting: SecuredBoolean!
+  isLanguageOfReporting: SecuredBoolean!
+  isLanguageOfWiderCommunication: SecuredBoolean!
   isSignLanguage: SecuredBoolean!
 
   """Whether this language has a Least Of These grant."""
   leastOfThese: SecuredBoolean!
@@ -2443,8 +2450,11 @@
 }
 
 input LanguageFilters {
   isDialect: Boolean
+  isLanguageOfConsulting: Boolean
+  isLanguageOfReporting: Boolean
+  isLanguageOfWiderCommunication: Boolean
   isSignLanguage: Boolean
 
   """Is a Least Of These partnership"""
   leastOfThese: Boolean
@@ -3219,8 +3229,9 @@
   globalInnovationsClient: SecuredBoolean!
 
   """The object's ID"""
   id: ID!
+  languageOfReporting: SecuredLanguageNullable!
   languageOfWiderCommunication: SecuredLanguageNullable!
 
   """Languages of the partner's affiliated translation projects"""
   languages(input: LanguageListInput = {count: 25, filter: {}, order: ASC, page: 1, sort: "name"}): SecuredLanguageList!
@@ -6726,8 +6737,11 @@
   ethnologue: UpdateEthnologueLanguage
   hasExternalFirstScripture: Boolean
   id: ID!
   isDialect: Boolean = false
+  isLanguageOfConsulting: Boolean
+  isLanguageOfReporting: Boolean
+  isLanguageOfWiderCommunication: Boolean
   isSignLanguage: Boolean
   leastOfThese: Boolean = false
   leastOfTheseReason: String
   name: String
@@ -6859,8 +6873,9 @@
   fieldRegions: [ID!]
   financialReportingTypes: [FinancialReportingType!]
   globalInnovationsClient: Boolean
   id: ID!
+  languageOfReportingId: ID
   languageOfWiderCommunicationId: ID
   languagesOfConsulting: [ID!]
   pmcEntityCode: String
   pointOfContactId: ID

⚠️ Dangerous Changes

  • An optional field isLanguageOfConsulting on input type CreateLanguage was added.
  • An optional field isLanguageOfReporting on input type CreateLanguage was added.
  • An optional field isLanguageOfWiderCommunication on input type CreateLanguage was added.
  • An optional field languageOfReportingId on input type CreatePartner was added.
  • An optional field isLanguageOfConsulting on input type LanguageFilters was added.
  • An optional field isLanguageOfReporting on input type LanguageFilters was added.
  • An optional field isLanguageOfWiderCommunication on input type LanguageFilters was added.
  • An optional field isLanguageOfConsulting on input type UpdateLanguage was added.
  • An optional field isLanguageOfReporting on input type UpdateLanguage was added.
  • An optional field isLanguageOfWiderCommunication on input type UpdateLanguage was added.
  • An optional field languageOfReportingId on input type UpdatePartner was added.

@adam-soltech adam-soltech force-pushed the agl/add-is-language-of-consultiung-boolean branch 2 times, most recently from ee65b1f to e7046f4 Compare December 7, 2023 23:18
Copy link
Contributor

@bryanjnelson bryanjnelson left a comment

Choose a reason for hiding this comment

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

Thanks @adam-soltech!

Copy link
Contributor

Choose a reason for hiding this comment

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

You wouldn't have known this, but we're not actually checking in EdgeDB Migrations at this point. Carson is handling this himself, and grouping them together as makes sense. Eventually that will change after we're fully migrated, but for now don't worry about doing the migration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without the migration the e2e tests failed

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes. Then for now just eliminate the EdgeDB additions completely. We'll attack that in a separate PR altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That also breaks the build. I sent the commit without edgedb and broken build to the dev channel

Copy link
Contributor

Choose a reason for hiding this comment

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

You'll still want to take both the .edgql and .esdl files out of this PR. Edge stuff is being done separately.

@CarsonF
Copy link
Member

CarsonF commented Dec 11, 2023

Language.isLanguageOfConsulting seems not the best name since it has the object name in it again.

  • isUsedForConsulting?
  • canBeUsedForConsulting?
  • allowedForConsulting?
  • consulting?

maybe an enum list would be better to encompass all of these?

Language.allowedUse = ['WiderCommunication', 'Consulting', 'X'];

Though I guess it would be better to describe what this language is vs how the language is allowed to be used in other places.
If we wanted different roles/conditions to be able to assign individual values, then a single list isn't the best way.

@bryanjnelson
Copy link
Contributor

Language.isLanguageOfConsulting seems not the best name since it has the object name in it again.

  • isUsedForConsulting?
  • canBeUsedForConsulting?
  • allowedForConsulting?
  • consulting?

maybe an enum list would be better to encompass all of these?

Language.allowedUse = ['WiderCommunication', 'Consulting', 'X'];

Though I guess it would be better to describe what this language is vs how the language is allowed to be used in other places. If we wanted different roles/conditions to be able to assign individual values, then a single list isn't the best way.

Yeah, after some thought I don't think the allowedUse is the way to go. Which means we're left with a naming decision...I'm leaning toward one of these depending on if we are set on the is prefix or not:

  • isUsedForConsulting
  • usedForConsulting

We seem to be inconsistent on the use of is, so I'm not sure what is best here...but those are my thoughts.

@adam-soltech
Copy link
Contributor Author

Though I guess it would be better to describe what this language is vs how the language is allowed to be used in other places. If we wanted different roles/conditions to be able to assign individual values, then a single list isn't the best way.

Also I'd note that a language might be a language of consulting as well as a language of reporting or wider communication so I'm not sure an enum is the way to go. isUsedForConsulting seems better and non-redundant. If that seems good to you let me know and I will make the update

@CarsonF
Copy link
Member

CarsonF commented Dec 12, 2023

isUsed seems like the least offensive prefix. I wonder if isUsedWithConsultants would be better than ForConsulting?

@bryanjnelson
Copy link
Contributor

isUsed seems like the least offensive prefix. I wonder if isUsedWithConsultants would be better than ForConsulting?

If the UI is going to say "for consulting" then I'd just as soon match that. Not much is gained otherwise imo.

@CarsonF
Copy link
Member

CarsonF commented Dec 12, 2023

I guess that's a question for product.

@adam-soltech
Copy link
Contributor Author

isUsed seems like the least offensive prefix. I wonder if isUsedWithConsultants would be better than ForConsulting?

If the UI is going to say "for consulting" then I'd just as soon match that. Not much is gained otherwise imo.

I agree but will run it by product in standup tomorrow

@adam-soltech adam-soltech force-pushed the agl/add-is-language-of-consultiung-boolean branch 2 times, most recently from c2e7bf4 to 7820dfa Compare January 26, 2024 19:15
@adam-soltech adam-soltech force-pushed the agl/add-is-language-of-consultiung-boolean branch from 7820dfa to e1907a2 Compare March 29, 2024 23:21
fix: subquery for partner

fix: subquery for partner
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.

None yet

3 participants