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

Pc 478 meerdere contazctmomenten registers klant #1042

Open
wants to merge 12 commits into
base: OudNaastNieuw
Choose a base branch
from

Conversation

githubjimmeicatt
Copy link
Contributor

No description provided.

@githubjimmeicatt githubjimmeicatt changed the base branch from OudNaastNieuw to pc-394-MeerdereContactmomentregisters February 3, 2025 16:29
@githubjimmeicatt githubjimmeicatt changed the base branch from pc-394-MeerdereContactmomentregisters to OudNaastNieuw February 4, 2025 09:58
)
);
}
// export function searchKlantenByDigitaalAdres(
Copy link
Contributor Author

@githubjimmeicatt githubjimmeicatt Feb 10, 2025

Choose a reason for hiding this comment

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

idem


function searchKlanten(url: string): Promise<PaginatedResult<Klant>> {
return fetchLoggedIn(url)
// export function useSearchKlanten({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

deze wordt volgens mij niet meer gebruikt maar even uitgecomment voor de zekerheid

Copy link
Contributor

Choose a reason for hiding this comment

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

haal maar weg dan :)

getBsn: () => string | undefined,
): ServiceData<Klant | null> {
const getUrl = () => getKlantBsnUrl(getBsn());
// export function useKlantByBsn(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

idem

fetchKlantByIdOk1,
);
}
// export function useKlantById(id: Ref<string>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

idem

Kiss.Bff/Extern/RegistryConfigExtensions.cs Show resolved Hide resolved

yield return new RegistrySystem
var isDefault = bool.TryParse(GetValue("IS_DEFAULT"), out var defaultValue) && defaultValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

ik weet niet hoe dat of dat in de vorige versie goed werd afgedekt (de compare is wat onoverzichtelijk), maar er mag maar 1 default zijn. anders svp een duidelijke error throwen

Copy link
Contributor

Choose a reason for hiding this comment

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

ik zie dat dat verderop in de validatie methode geregeld wordt.

if (systemen.Count == 0) return "";
if (systemen.Count == 0)
{
return "FOUT: Er zijn geen registraties geconfigureerd.";
Copy link
Contributor

Choose a reason for hiding this comment

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

misschien een iets behulpzamere tekst? bv: Er zijn geen registraties voor klanten, contactmomenten en/of contactverzoeken geconfigureerd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aangepast

Kiss.Bff/Extern/RegistryConfigExtensions.cs Show resolved Hide resolved
registryVersions,
} from "@/services/environment/fetch-systemen";

export const getRegisterDetails = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

ik vind de naamgeving 'register' zowel in front als backend niet helemaal ideaal. te algemeen. het brp is ook een register. het gaat hier specifiek om klantcontact registers.
om alles niet teveel door elkaar te laten lopen kunnen we misschien na deze pr een losse pr met wat naamswijzigingen doorvoeren

}

return {
systeemId: defaultSysteem.identifier,
Copy link
Contributor

Choose a reason for hiding this comment

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

dit is verwarrend. svp hernoemen naar defaultSysteemId


function searchKlanten(url: string): Promise<PaginatedResult<Klant>> {
return fetchLoggedIn(url)
// export function useSearchKlanten({
Copy link
Contributor

Choose a reason for hiding this comment

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

haal maar weg dan :)

export const ensureKlantForBsn = async (
parameters: { bsn: string }
) => {
export const ensureKlantForBsn = async (parameters: { bsn: string }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

dit is een beetje een raar geval, inconsistent en merkwaardige functies createklant2 en ensureKlantForBsn1.
kunnen we deze opheffen en de code iets strakker maken (zoals hieronder) en rechtstreeks opnemen in personenOverzicht.vue?

const ensureKlantForBsn = async (parameters: { bsn: string }) => {
  const { useKlantInteractiesApi, defaultSysteemId } = await getRegisterDetails();

 const klant  = (useKlantInteractiesApi) 
 ? await ensureOk2Klant(systeemId, parameters);
 : await ensureOk1Klant(systeemId, parameters, useBronOrganisatie());



{
Destination = destination;
_tokenProvider = tokenProvider;
var registry = string.IsNullOrWhiteSpace(systemIdentifier)
Copy link
Contributor

Choose a reason for hiding this comment

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

sturen we hem niet altijd mee?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aangepast

Kiss.Bff/Extern/RegistryConfig.cs Show resolved Hide resolved
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.

3 participants