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

Fix properties not being displayed (needs testing) #3644

Merged
merged 1 commit into from
Oct 16, 2023
Merged

Fix properties not being displayed (needs testing) #3644

merged 1 commit into from
Oct 16, 2023

Conversation

j-maas
Copy link
Contributor

@j-maas j-maas commented Oct 3, 2023

This should fix #3440.

When opening a contact with custom labels as in the issue, I noticed the following error message in the console:

TypeError: this.properties[this.property.name] is undefined
    isMultiple ContactDetailsProperty.vue:136
    VueJS 3
    jN ContactDetailsProperty.vue:5
    render vue-composition-api.common.prod.js:1
    Qe vue-composition-api.common.prod.js:1
    render vue-composition-api.common.prod.js:1
    VueJS 32
    r ContactDetails.vue:805
    c isTalkEnabled.js:25
    y isTalkEnabled.js:25
    b isTalkEnabled.js:25
    eR isTalkEnabled.js:25
    o isTalkEnabled.js:25
    promise callback*eR isTalkEnabled.js:25
    o isTalkEnabled.js:25
    promise callback*eR isTalkEnabled.js:25
    o isTalkEnabled.js:25
    tR isTalkEnabled.js:25
    tR isTalkEnabled.js:25
    selectContact ContactDetails.vue:770
    contact ContactDetails.vue:684
    VueJS 11
    init vue-router.esm.js:3005
    init vue-router.esm.js:3004
    updateRoute vue-router.esm.js:2414
    transitionTo vue-router.esm.js:2263
    confirmTransition vue-router.esm.js:2402
    r vue-router.esm.js:2084
    Qe vue-router.esm.js:2095
    confirmTransition vue-router.esm.js:2396
    r vue-router.esm.js:2084
    r vue-router.esm.js:2088
    g vue-router.esm.js:2384
    Ke vue-router.esm.js:2162
    g vue-router.esm.js:2362
    r vue-router.esm.js:2087
    r vue-router.esm.js:2091
    r vue-router.esm.js:2091
    Qe vue-router.esm.js:2095
    confirmTransition vue-router.esm.js:2392
    transitionTo vue-router.esm.js:2260
    push vue-router.esm.js:2606
    push vue-router.esm.js:3039
    _ vue-router.esm.js:1139
    onClick index.module.js:2

Since in PR #2064 there was a similar issue in another method, I suspect that the same fix can be applied here.

However, I was unfortunately not able to test this change, since I don't have a full development setup and I couldn't hack it in the browser. So you need to test that this fix is correct.

@j-maas j-maas changed the title Fix missing properties (needs testing) Fix properties not being displayed (needs testing) Oct 3, 2023
Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

src/components/ContactDetails/ContactDetailsProperty.vue Outdated Show resolved Hide resolved
@kesselb kesselb added bug Something isn't working 3. to review Waiting for reviews labels Oct 10, 2023
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (38db5d0) 1.81% compared to head (00de88b) 2.07%.
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main   #3644      +/-   ##
==========================================
+ Coverage     1.81%   2.07%   +0.26%     
==========================================
  Files          113      89      -24     
  Lines         6130    5354     -776     
  Branches      1491    1492       +1     
==========================================
  Hits           111     111              
+ Misses        5901    5125     -776     
  Partials       118     118              
Files Coverage Δ
...mponents/ContactDetails/ContactDetailsProperty.vue 0.00% <ø> (ø)

... and 24 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@j-maas
Copy link
Contributor Author

j-maas commented Oct 14, 2023

@kesselb I'm a bit overwhelmed with what exactly to do. If you tell me what to do, I might try, but I currently don't have much time to dig in.

So if you're dependent on me doing something, it might be easier to open a new PR... I hope that's ok with you.

@st3iny
Copy link
Member

st3iny commented Oct 16, 2023

I simplified the code even further.

@j-maas Thank you for the PR.

Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Tested and works.

@st3iny st3iny added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 16, 2023
@st3iny st3iny added this to the v5.5.0 milestone Oct 16, 2023
@st3iny
Copy link
Member

st3iny commented Oct 16, 2023

/backport to stable5.4

@st3iny st3iny merged commit c40864d into nextcloud:main Oct 16, 2023
27 checks passed
@j-maas j-maas deleted the fix-missing-properties branch October 16, 2023 18:56
@j-maas
Copy link
Contributor Author

j-maas commented Oct 16, 2023

Thanks for taking this on! ❤️

@DiagonalArg
Copy link

Where should we see this? I'm on 5.5.0, everything updated, and I'm still getting addresses rendered improperly, as here: #3736 (comment)

@st3iny
Copy link
Member

st3iny commented Dec 20, 2023

@DiagonalArg This fix is about missing props in our frontend. Please open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish backport-request bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Phone field missing on many contacts
4 participants