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

Ability to have engagements where the Intern/Language is uncertain. #2834

Closed
wants to merge 5 commits into from

Conversation

elisabeth-sorrell
Copy link
Collaborator

@elisabeth-sorrell elisabeth-sorrell commented Jul 24, 2023

Closes SeedCompany/cord-field#1121

┆Issue is synchronized with this Monday item by Unito

…n that can be set when the specific intern/language is unknown for an engagement as of yet.
…l checks to update, and the rest of the CRUD operations and dtos handle nameWhenUnknown and optional language/intern ids.
@github-actions
Copy link

github-actions bot commented Jul 24, 2023

🗞 GraphQL Summary

View schema changes
@@ -559,9 +559,9 @@
   countryOfOriginId: ID
   disbursementCompleteDate: Date
   endDateOverride: Date
   growthPlan: CreateDefinedFileVersionInput
-  internId: ID!
+  internId: ID
   mentorId: ID
   methodologies: [ProductMethodology!]
   position: InternshipPosition
   projectId: ID!
@@ -605,9 +605,9 @@
   disbursementCompleteDate: Date
   endDateOverride: Date
   firstScripture: Boolean
   historicGoal: String
-  languageId: ID!
+  languageId: ID
   lukePartnership: Boolean
 
   """
   This is the methodology that will be set on products extracted out of the pnp.
@@ -1374,8 +1374,18 @@
   initialEndDate: SecuredDateNullable!
   lastReactivatedAt: SecuredDateTime!
   lastSuspendedAt: SecuredDateTime!
   modifiedAt: DateTime!
+
+  """
+  Automatically set when we know what kind of engagement this is, but not
+  specifically what language/intern this is engaged to.
+  This is only set when the id this is engaged to is not defined. 
+  For example, if this is a language engagement but the language is 
+  unknown, the language ID would be null and this value would be set.
+  Once an ID for language is set, this value would be set to null again.
+  """
+  nameWhenUnknown: SecuredStringNullable!
   parent: Project!
   project: Project!
 
   """Based on the project's sensitivity"""
@@ -1956,8 +1966,18 @@
   lastSuspendedAt: SecuredDateTime!
   mentor: SecuredUser!
   methodologies: SecuredMethodologies!
   modifiedAt: DateTime!
+
+  """
+  Automatically set when we know what kind of engagement this is, but not
+  specifically what language/intern this is engaged to.
+  This is only set when the id this is engaged to is not defined. 
+  For example, if this is a language engagement but the language is 
+  unknown, the language ID would be null and this value would be set.
+  Once an ID for language is set, this value would be set to null again.
+  """
+  nameWhenUnknown: SecuredStringNullable!
   parent: InternshipProject!
   position: SecuredInternPosition!
   project: Project!
 
@@ -2267,8 +2287,18 @@
   lukePartnership: SecuredBoolean!
   modifiedAt: DateTime!
 
   """
+  Automatically set when we know what kind of engagement this is, but not
+  specifically what language/intern this is engaged to.
+  This is only set when the id this is engaged to is not defined. 
+  For example, if this is a language engagement but the language is 
+  unknown, the language ID would be null and this value would be set.
+  Once an ID for language is set, this value would be set to null again.
+  """
+  nameWhenUnknown: SecuredStringNullable!
+
+  """
   The progress report due next. This is the period currently in progress.
   """
   nextProgressReportDue: SecuredProgressReport!
   openToInvestorVisit: SecuredBoolean!
@@ -6205,8 +6235,11 @@
   disbursementCompleteDate: Date
   endDateOverride: Date
   growthPlan: CreateDefinedFileVersionInput
   id: ID!
+
+  """Leave null/undefined if intern is not known yet."""
+  internId: ID
   mentorId: ID
   methodologies: [ProductMethodology!]
   position: InternshipPosition
   startDateOverride: Date
@@ -6252,8 +6285,11 @@
   endDateOverride: Date
   firstScripture: Boolean
   historicGoal: String
   id: ID!
+
+  """Leave null/undefined if language is not known yet."""
+  languageId: ID
   lukePartnership: Boolean
 
   """
   This is the methodology that will be set on products extracted out of the pnp.

⚠️ Dangerous Changes

  • An optional field internId on input type UpdateInternshipEngagement was added.
  • An optional field languageId on input type UpdateLanguageEngagement was added.

@elisabeth-sorrell
Copy link
Collaborator Author

elisabeth-sorrell commented Jul 24, 2023

Requirements: Need to be able to add a Language or Intern engagement without specifying the language or intern because there are times in a project when other engagement details are developing but the language or intern itself if unknown. There are also times when an incorrect language or intern is inadvertently added, and right now the only way to correct it is either deleting the engagement and creating a new one with the correct language or intern, or by going straight to the db to establish the correct connections.

The solution to these problems is by letting a user create an engagement without necessarily specifying the intern or language, and then letting the user update the engagement later on in the engagement’s detail page. When asked about how to distinguish between multiple unknown engagements, Seth said he wanted some sort of arbitrarily generated name for each unknown language/intern (perhaps Language 1,2,3, Intern 1,2,3 or Language A,B,C etc).

  • Make language and intern ids optional for engagements
    Not so much of a big deal, just change the dtos. I also thought of maybe creating some newer abstract dtos like Unknown[Language|Intern]Engagement and then extending those with concrete classes [Language|Intern]Engagement. I wasn't really sure though about how much that really brought to the table other than confusion. So far, just making the ids optional seemed the most straightforward way for now.

  • Once they are optional, each of the CRUD operations need to be double-checked and tweaked to handle an optional Language/Intern id
    For instance, if a [Language|Intern] engagement is being updated, we need to make sure that we aren't updating to a duplicated [Language|Intern]. Also the queries need double checking to make sure that just adding a ? to the ids would do the trick

  • Find a way to identify each unknown engagement, I decided to go with the third idea below, because it doesn't seem to be very confusing. I don't really like the name nameWhenUnknown though... :
    I thought of a few ways we could do this...

    • We could make the [language|intern]ids a unioned type of ID|Unknown_ID, where the ID is the usual generated id that we do for neo4j and the Unknown_ID looks like [Language|Intern] ${SOME_DIGIT}. Some caveats I thought of doing that would mean that we have to make a parser to distinguish between known/unknown ids, we also don't ever do ids like that, also matching would just turn into a mess because the unknown ids would actually return a result when we don't want to have one, not to mention the fact that we use that id to match a [Language|Intern] node.
    • We could let users define their own unknown Language/Intern engagement names. Not sure how that would look like graphically, and doesn't seem right to have a prop hanging around that's going to be dead half the time. And, it kind of defeats Seth's idea of auto-generating them.
    • We could add a calculated prop, nameWhenUnknown, that's generated when a user doesn't specify a language\intern. I don't really like the name, but I think we do need to somehow persist an unknown name in the db, and this seems to get the job done
    • Could have the front-end just generate names on the fly, without persisting the value at all. Not really into that, because for one thing, I don't think it's good practice to have the front end responsible for generating data, and for another thing, it's going to get confusing once users start going from unknown to known engagements.
  • identify and implement other business rules when it comes to having an unknown engagement (eg, what steps are allowed/ not allowed to have unknown engagements, etc)

    • Adding unknown Engagements are only allowed when Project is InDevelopment
    • A User is not allowed to transition to Pending Consultant Endorsement if there are unknown engagements.
    • All other business rules for adding engagements still apply

…rep to pending consultant endorsement that there are no unknown engagements.
@elisabeth-sorrell elisabeth-sorrell marked this pull request as ready for review July 26, 2023 15:22
@elisabeth-sorrell
Copy link
Collaborator Author

@CarsonF Ready at your convenience.

@sethmcknight
Copy link
Member

This lived to the end of its lifetime and died

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.

Anticipated Engagements Count
2 participants