Skip to content

Add acceptance test for EE Ticket #3241#325

Closed
micbar wants to merge 1 commit into
masterfrom
fix/uppercasemail
Closed

Add acceptance test for EE Ticket #3241#325
micbar wants to merge 1 commit into
masterfrom
fix/uppercasemail

Conversation

@micbar

@micbar micbar commented Apr 18, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Fix for owncloud/enterprise#3241

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.

@micbar
micbar force-pushed the fix/uppercasemail branch from 43e9105 to 6c98ab3 Compare April 18, 2019 12:36
@codecov

codecov Bot commented Apr 18, 2019

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #325   +/-   ##
=========================================
  Coverage     17.87%   17.87%           
  Complexity      101      101           
=========================================
  Files            11       11           
  Lines           537      537           
=========================================
  Hits             96       96           
  Misses          441      441

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd076fc...6c98ab3. Read the comment docs.

@codecov

codecov Bot commented Apr 18, 2019 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #325   +/-   ##
=========================================
  Coverage     17.87%   17.87%           
  Complexity      101      101           
=========================================
  Files            11       11           
  Lines           537      537           
=========================================
  Hits             96       96           
  Misses          441      441
Impacted Files Coverage Δ Complexity Δ
lib/Controller/RegisterController.php 0% <0%> (ø) 16 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5b6961...766281f. Read the comment docs.

@micbar
micbar force-pushed the fix/uppercasemail branch 2 times, most recently from 817d464 to 2f03118 Compare April 23, 2019 14:21
@micbar micbar changed the title Add accenptance test for EE Ticket #3241 Add acceptance test for EE Ticket #3241 Apr 23, 2019
@micbar
micbar force-pushed the fix/uppercasemail branch from 2f03118 to 766281f Compare May 3, 2019 07:26
@PVince81
PVince81 requested a review from individual-it May 3, 2019 13:55
@PVince81 PVince81 added this to the development milestone May 3, 2019
@PVince81

PVince81 commented May 3, 2019

Copy link
Copy Markdown
Contributor

@individual-it @phil-davis please review

@micbar micbar mentioned this pull request May 6, 2019
Closed
11 tasks
@micbar

micbar commented May 6, 2019

Copy link
Copy Markdown
Contributor Author

@phil-davis @individual-it The core PR owncloud/core#35156 is needed to make this work.

individual-it
individual-it reviewed May 7, 2019
View reviewed changes
Comment thread tests/acceptance/features/bootstrap/GuestsContext.php
* @return string
*/
public function prepareUserNameAsFrontend($guestEmail) {
return \strtolower(\trim(\urldecode($guestEmail)));

@individual-it individual-it May 7, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why do we need that change?

@micbar micbar May 7, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we always convert the UserID to lowercase, we cannot test real world scenarios where the users enter Uppercase letters as part of the E-Mail Address.

@micbar

micbar commented May 31, 2019

Copy link
Copy Markdown
Contributor Author

Needs to be decided. There are issues with the lowercase usernames.

@micbar

micbar commented Jun 7, 2019

Copy link
Copy Markdown
Contributor Author

@phil-davis let us close this.
The case is not easy to test, because the fix is in the UI / JS which converts the e-mail to lowercase before making the request to the sharing api.

@micbar micbar closed this Jun 7, 2019
@micbar
micbar deleted the fix/uppercasemail branch September 25, 2019 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@individual-it individual-it individual-it approved these changes

Assignees

@individual-it individual-it

Labels

3 - To review enhancement QA

Projects

None yet

Milestone

development

Development

Successfully merging this pull request may close these issues.

3 participants

@micbar @PVince81 @individual-it