Skip to content

Conversation

@ChristophWurst
Copy link
Member

Follow-up to #2954
Discussed in #2726 (comment)

Previously assigned managers will show a broken link because we wrongly stored the UID as display name. Those contacts need to be fixed but it only affects dev and pre-production instances.

@ChristophWurst ChristophWurst added the 2. developing Work in progress label Sep 23, 2022
@ChristophWurst ChristophWurst added this to the v5.0 milestone Sep 23, 2022
@ChristophWurst ChristophWurst self-assigned this Sep 23, 2022
@codecov
Copy link

codecov bot commented Sep 23, 2022

Codecov Report

Base: 31.21% // Head: 67.96% // Increases project coverage by +36.74% 🎉

Coverage data is based on head (7bdddc8) compared to base (f388856).
Patch has no changes to coverable lines.

❗ Current head 7bdddc8 differs from pull request most recent head 240ee34. Consider uploading reports for the commit 240ee34 to get more accurate results

Additional details and impacted files
@@              Coverage Diff              @@
##               main    #2962       +/-   ##
=============================================
+ Coverage     31.21%   67.96%   +36.74%     
  Complexity      253      253               
=============================================
  Files           109       23       -86     
  Lines          1858      721     -1137     
  Branches        218        0      -218     
=============================================
- Hits            580      490       -90     
+ Misses         1163      231      -932     
+ Partials        115        0      -115     
Impacted Files Coverage Δ
src/components/AppContent/ChartContent.vue
...mponents/ContactDetails/ContactDetailsProperty.vue
src/components/EntityPicker/EntityPicker.vue
src/components/OrgChart.vue
src/models/contact.js
src/utils/chartUtils.js
src/mixins/RouterMixin.js
src/services/cdav.js
src/views/Processing/ImportView.vue
src/components/EntityPicker/ContactsPicker.vue
... and 76 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ChristophWurst
Copy link
Member Author

Seems to do the trick now

@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 23, 2022
@ChristophWurst ChristophWurst marked this pull request as ready for review September 23, 2022 14:27
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.

Code looks good otherwise.

// TODO: this only *shows* the display name but doesn't assign the missing UID
const displayName = this.property.getFirstValue()
const other = this.otherContacts(this.contact).filter(contact => contact.displayName === displayName)
return other?.key
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return other?.key
return other.key

The variable other is always defined because Array.filter in the line above always returns an array.

Copy link
Member Author

Choose a reason for hiding this comment

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

This actually shows a problem. I meant to use .find instead of .filter. Then the result is undefind|object :)

@ChristophWurst ChristophWurst force-pushed the fix/org-chart-save-uid-and-display-name branch from c79e597 to 240ee34 Compare September 27, 2022 17:12
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Sep 27, 2022
@ChristophWurst ChristophWurst merged commit e058254 into main Sep 27, 2022
@ChristophWurst ChristophWurst deleted the fix/org-chart-save-uid-and-display-name branch September 27, 2022 17:36
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

Projects

Development

Successfully merging this pull request may close these issues.

3 participants