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

Psp 7281 #3617

Merged
merged 6 commits into from
Nov 25, 2023
Merged

Psp 7281 #3617

merged 6 commits into from
Nov 25, 2023

Conversation

devinleighsmith
Copy link
Collaborator

No description provided.

@devinleighsmith devinleighsmith added the enhancement New feature or request label Nov 24, 2023
@devinleighsmith devinleighsmith self-assigned this Nov 24, 2023
Comment on lines 38 to 41
.Map(dest => dest.DispositionFundingTypeCodeNavigation, src => src.FundingTypeCode)
.Map(dest => dest.DispositionFileStatusTypeCodeNavigation, src => src.FileStatusTypeCode)
.Map(dest => dest.DspPhysFileStatusTypeCodeNavigation, src => src.PhysicalFileStatusTypeCode)
.Map(dest => dest.DispositionTypeCodeNavigation, src => src.DispositionTypeCode)
Copy link
Collaborator

@asanchezr asanchezr Nov 24, 2023

Choose a reason for hiding this comment

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

just checking that we have a mapping that goes from model to the typecode navigation...
I think we usually set the string on the way in, not the navigation property itself

ie

.Map(dest => dest.DispositionTypeCode, src => src.DispositionTypeCode.Id)
...


/*
* Frontend model
* LINK @frontend/src\models\api\DispositionFile.ts:14
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit and question: are we forced to use a mix of forward slashes and backward slashes here or can we use forward slashes only?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would be a better question for @FuriousLlama, as currently he is the only one with this library functional. This conforms to the pattern that I've seen on other backend model files.

Comment on lines +19 to +20
.Map(dest => dest.TeamProfileType, src => src.DspFlTeamProfileTypeCodeNavigation)
.Map(dest => dest.TeamProfileTypeCode, src => src.DspFlTeamProfileTypeCode)
Copy link
Collaborator

Choose a reason for hiding this comment

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

TeamProfile and TeamProfileTypeCode seems to be duplicated (one is the string code, the other the typemodel representation of it). Perhaps the string one could be removed?

Comment on lines +52 to +60
/// <summary>
/// get/set - The Team's profile type code.
/// </summary>
public string TeamProfileTypeCode { get; set; }

/// <summary>
/// get/set - The Team's profile type code.
/// </summary>
public TypeModel<string> TeamProfileType { get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a need to keep the duplicate TeamProfileTypeCode in the model when we already have the TypeModel for it?

.ThenInclude(d => d.PrimaryContact)
.Include(d => d.PimsDispositionFileTeams)
.ThenInclude(d => d.DspFlTeamProfileTypeCodeNavigation)
.FirstOrDefault(d => d.DispositionFileId == id);
Copy link
Collaborator

Choose a reason for hiding this comment

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

want to confirm this follows existing patterns - I believe we throw a keynotfound exception if no entity is found by id?

@@ -1,4 +1,5 @@
using System.Collections.Generic;
using DocumentFormat.OpenXml.InkML;
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this import needed?

<StyledFormWrapper>
{error && (
<b>
Failed to load Diposition File. Check the detailed error in the top right for
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: type "Diposition" -> Disposition

Comment on lines +50 to +51
teamProfileTypeCode?: string;
teamProfileType?: Api_TypeCode<string>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

like the backend models I think the type model is enough - no need to keep the string code as well

@devinleighsmith devinleighsmith merged commit 4fd93c5 into bcgov:disposition Nov 25, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants