Skip to content
This repository has been archived by the owner on Jun 8, 2023. It is now read-only.

feat: associate contacts with posts #245

Merged
merged 22 commits into from
Dec 4, 2022
Merged

Conversation

djaiss
Copy link
Member

@djaiss djaiss commented Oct 10, 2022

This PR lets you associate a contact with a post.

You can associate as many contacts in a post as you want.

This uses the ContactSelector component that is also used on the Loan widget.

@djaiss djaiss added the draft label Oct 10, 2022
@djaiss djaiss marked this pull request as ready for review December 3, 2022 21:30
@djaiss
Copy link
Member Author

djaiss commented Dec 3, 2022

@asbiin I need your help. I don't understand what's happening inside the ContactSelector component.

Basically, inside the Edit view, the Contacts are passed as ModelValue to the ContactSelector component. Inside the component, the ModelValue instantiates an array called localContacts which is used to populate the list of contacts.

However, on page load, the contacts are not displayed, because the component thinks the localContacts variable is empty, even though ModelValue is not empty.

I have no idea what's going on and why this is happening.

@djaiss djaiss marked this pull request as draft December 3, 2022 21:35
@asbiin
Copy link
Member

asbiin commented Dec 3, 2022

However, on page load, the contacts are not displayed, because the component thinks the localContacts variable is empty, even though ModelValue is not empty.

@djaiss yes on page load == when vue run mounted, the value of the modelValue isn't set yet. You need to add a watch method on the modelValue to set the value of the local variable when modelValue changes.

@djaiss djaiss marked this pull request as ready for review December 3, 2022 23:42
@djaiss djaiss requested a review from asbiin December 4, 2022 00:45
Copy link
Member

@asbiin asbiin left a comment

Choose a reason for hiding this comment

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

I'm proposing some small changes, but otherwise it's great!

@@ -285,6 +286,10 @@ public static function modules(TemplatePage $page, Contact $contact, User $user)
$data = ModulePhotosViewHelper::data($contact);
}

if ($module->type == Module::TYPE_POSTS) {
Copy link
Member

Choose a reason for hiding this comment

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

I would have used a switch on the whole foreach, but it's more changes

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. I won't do this in this PR but will do it. It'll be faster.

djaiss and others added 2 commits December 4, 2022 12:59
…t.php

Co-authored-by: Alexis Saettler <alexis@saettler.org>
Co-authored-by: Alexis Saettler <alexis@saettler.org>
@djaiss djaiss enabled auto-merge (squash) December 4, 2022 18:01
@sonarcloud
Copy link

sonarcloud bot commented Dec 4, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

82.3% 82.3% Coverage
5.0% 5.0% Duplication

@djaiss djaiss merged commit bedc777 into main Dec 4, 2022
@djaiss djaiss deleted the 2022-10-10-contact-in-posts branch December 4, 2022 18:08
asbiin pushed a commit to monicahq/monica that referenced this pull request Mar 31, 2023
asbiin pushed a commit to monicahq/monica that referenced this pull request May 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants