Remove contact id in share with email code #27225

Merged
merged 1 commit into from Feb 28, 2017

Conversation

Projects
None yet
3 participants
@PVince81
Member

PVince81 commented Feb 23, 2017

Description

The id is not used and would cause warnings when results come from the
system address book.

Related Issue

N/A

Motivation and Context

How Has This Been Tested?

Steps to reproduce:

  1. Setup LDAP users with LDAP docker: https://github.com/owncloud/administration/tree/master/ldap-testing
  2. Setup LDAP in ownCloud and set "mail" attribute in the wizard (you might need to do it twice due to this annoying bug owncloud/user_ldap#22)
  3. Run occ dav:sync-system-addressbook
  4. Go to admin page, untick then tick again the setting "Allow username autocompletion in share dialog." (because of this bug #27224)
  5. Tick "Allow users to send mail notification for shared files"
  6. Go to files list
  7. Share a file with link
  8. Type in "zom" in the email link field to autocomplete zombie email addresses
  9. Check log

This message repeats for every result:

{"reqId":"XJyzTH3u73t6NhHOdyXr","remoteAddr":"127.0.0.1","app":"PHP","message":"Undefined index: id at \/srv\/www\/htdocs\/owncloud\/core\/ajax\/share.php#233","level":3,"time":"2017-02-23T10:48:40+00:00","method":"GET","url":"\/owncloud\/index.php\/core\/ajax\/share.php?fetch=getShareWithEmail&search=zom","user":"admin"}

After the fix: no more warning messages.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@DeepDiver1975 @jvillafanez @VicDeo @IljaN please review

Remove contact id in share with email code
The id is not used and would cause warnings when results come from the
system address book.
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Feb 23, 2017

@PVince81, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975, @bartv2 and @frisco82 to be potential reviewers.

@PVince81, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975, @bartv2 and @frisco82 to be potential reviewers.

@jvillafanez

This comment has been minimized.

Show comment
Hide comment
@jvillafanez

jvillafanez Feb 27, 2017

Member

I think this is fine if the issue is limited to the web UI. I don't know if webDAV or other interfaces are also affected by this issue, and if this is the case removing the id attribute might not be a good choice.

Member

jvillafanez commented Feb 27, 2017

I think this is fine if the issue is limited to the web UI. I don't know if webDAV or other interfaces are also affected by this issue, and if this is the case removing the id attribute might not be a good choice.

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Feb 27, 2017

Member

@jvillafanez this is the old cruddy private "ajax/share.php" "API" that must die soon.
Only the web UI calls this so this should be fine.

Member

PVince81 commented Feb 27, 2017

@jvillafanez this is the old cruddy private "ajax/share.php" "API" that must die soon.
Only the web UI calls this so this should be fine.

@jvillafanez

This comment has been minimized.

Show comment
Hide comment
@jvillafanez

jvillafanez Feb 28, 2017

Member

Ok then 👍

Member

jvillafanez commented Feb 28, 2017

Ok then 👍

@PVince81 PVince81 merged commit 520f151 into master Feb 28, 2017

3 of 4 checks passed

continuous-integration/jenkins/pr-head This commit is being built
Details
Scrutinizer 10 new issues, 7 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@PVince81 PVince81 deleted the search-email-removeid branch Feb 28, 2017

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 6, 2017

Member

stable9.1: #27317

Member

PVince81 commented Mar 6, 2017

stable9.1: #27317

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment