Fixed different app author scenarios not working #29381

splitt3r · 2017-10-28T16:11:27Z

Description

If there are multiple app authors or the app author adds email / url to tag in info.xml oC is not able to display the information correctly.

Examples:

Related Issue

PR #29221 is somehow related.

How Has This Been Tested?

Tested in FF, Chrome and Edge

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.

CLAassistant · 2017-10-28T16:11:34Z

All committers have signed the CLA.

codecov · 2017-10-28T16:37:54Z

Codecov Report

Merging #29381 into master will increase coverage by 1.05%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master   #29381      +/-   ##
============================================
+ Coverage     60.21%   61.27%   +1.05%     
- Complexity    17188    17472     +284     
============================================
  Files          1030     1044      +14     
  Lines         57274    57676     +402     
============================================
+ Hits          34490    35343     +853     
+ Misses        22784    22333     -451

Impacted Files	Coverage Δ	Complexity Δ
lib/private/DB/ConnectionFactory.php	`60.52% <0%> (-2.49%)`	`21% <0%> (ø)`
lib/private/DB/Connection.php	`63.43% <0%> (-2.46%)`	`48% <0%> (+3%)`
lib/private/AvatarManager.php	`58.33% <0%> (-1.67%)`	`8% <0%> (ø)`
lib/private/User/Session.php	`62.13% <0%> (-1.02%)`	`124% <0%> (+5%)`
core/Application.php	`27% <0%> (-1%)`	`2% <0%> (ø)`
lib/private/Config.php	`88.04% <0%> (-0.85%)`	`38% <0%> (+1%)`
apps/files_sharing/lib/AppInfo/Application.php	`44.26% <0%> (-0.74%)`	`4% <0%> (ø)`
lib/private/Files/Storage/Wrapper/Encryption.php	`67.82% <0%> (-0.7%)`	`154% <0%> (+3%)`
lib/private/legacy/api.php	`41.7% <0%> (-0.18%)`	`81% <0%> (+1%)`
core/Controller/LostController.php	`88.09% <0%> (-0.15%)`	`32% <0%> (+2%)`
... and 63 more

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 98e73e5...04d8ed7. Read the comment docs.

DeepDiver1975 · 2017-10-30T07:37:02Z

settings/js/admin-apps.js

+			app.author = app.author.join(', ');
+		}
+
+		if (_.isObject(app.author) && !_.isUndefined(app.author['@value'])) {


does this work if app.author is an array of objects?

Good point 😁. I will ivestigate and update the code.

PVince81 · 2017-12-19T16:37:19Z

would be good to add a JS unit test, at least of the parse author logic and its two code paths.

You can use make test-js-debug

splitt3r · 2017-12-19T16:52:08Z

Ok, i will have a look at it.

PVince81

👍 thanks

PVince81 · 2018-01-08T11:18:28Z

I've restarted the Travis tests, let's hope it was a one-time thing

splitt3r · 2018-01-08T12:01:58Z

Hm doesn´t look that good 😞. Are these errors related to my changes?

phil-davis · 2018-01-08T12:39:07Z

The Travis tests that fail are the UI tests - that is because this PR is from a forked repo, so the SauceLabs credentials are not available to the job.

splitt3r · 2018-01-08T12:42:59Z

Thanks for the clarification :)

phil-davis · 2018-01-08T12:47:21Z

I pushed this branch into my fork, where I have my own SauceLabs credentials.
https://github.com/phil-davis/core/pull/135
That is running the UI tests now and will demonstrate (hopefully!) that they pass.

phil-davis · 2018-01-08T14:02:07Z

@splitt3r @PVince81 Travis UI tests passed in my fork. Suitable god privs could be used to merge this PR, overriding the Travis fail here.

PVince81 · 2018-01-08T14:04:34Z

@splitt3r can you backport to stable10 ? Basically create a branch off stable10, cherry pick your commits and send a PR from that branch to stable10.

If not let us know and we'll take care.

phil-davis · 2018-01-08T14:20:25Z

Care taken :)
Backport stable10 #30043
It is easy to do it in the "real" core repo - saves the hassle of UI test fails.

lock · 2019-08-01T13:02:04Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Fixed different app author scenarios not working

22820e9

splitt3r added 2 commits October 28, 2017 18:21

Fixed app search by author

8ec2c78

Unified Underscore.js usage

514838b

Updated search by author statement

347883d

DeepDiver1975 added this to the development milestone Oct 30, 2017

DeepDiver1975 added the 3 - To Review label Oct 30, 2017

DeepDiver1975 requested changes Oct 30, 2017

View reviewed changes

PVince81 modified the milestones: development, planned Nov 8, 2017

Added support for mixed arrays of authors with or without extra info

f765aed

PVince81 modified the milestones: planned, development Nov 22, 2017

ownclouders added status/STALE and removed status/STALE labels Dec 7, 2017

ownclouders removed the status/STALE label Dec 19, 2017

ownclouders added the status/STALE label Dec 27, 2017

Added basic unit tests

04d8ed7

ownclouders removed the status/STALE label Jan 4, 2018

PVince81 approved these changes Jan 8, 2018

View reviewed changes

PVince81 added 4 - To release backport-request and removed 3 - To Review labels Jan 8, 2018

PVince81 merged commit e668bea into owncloud:master Jan 8, 2018

phil-davis mentioned this pull request Jan 8, 2018

[stable10] fix app author #30043

Merged

phil-davis removed the backport-request label Jan 8, 2018

splitt3r deleted the fix-app-author branch January 8, 2018 18:39

lock bot locked as resolved and limited conversation to collaborators Aug 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed different app author scenarios not working #29381

Fixed different app author scenarios not working #29381

splitt3r commented Oct 28, 2017

CLAassistant commented Oct 28, 2017 •

edited

Loading

codecov bot commented Oct 28, 2017 •

edited

Loading

DeepDiver1975 Oct 30, 2017

splitt3r Oct 30, 2017 •

edited

Loading

PVince81 commented Dec 19, 2017

splitt3r commented Dec 19, 2017

PVince81 left a comment

PVince81 commented Jan 8, 2018

splitt3r commented Jan 8, 2018

phil-davis commented Jan 8, 2018

splitt3r commented Jan 8, 2018

phil-davis commented Jan 8, 2018

phil-davis commented Jan 8, 2018

PVince81 commented Jan 8, 2018

phil-davis commented Jan 8, 2018

lock bot commented Aug 1, 2019

Fixed different app author scenarios not working #29381

Fixed different app author scenarios not working #29381

Conversation

splitt3r commented Oct 28, 2017

Description

Related Issue

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

CLAassistant commented Oct 28, 2017 • edited Loading

codecov bot commented Oct 28, 2017 • edited Loading

Codecov Report

DeepDiver1975 Oct 30, 2017

Choose a reason for hiding this comment

splitt3r Oct 30, 2017 • edited Loading

Choose a reason for hiding this comment

PVince81 commented Dec 19, 2017

splitt3r commented Dec 19, 2017

PVince81 left a comment

Choose a reason for hiding this comment

PVince81 commented Jan 8, 2018

splitt3r commented Jan 8, 2018

phil-davis commented Jan 8, 2018

splitt3r commented Jan 8, 2018

phil-davis commented Jan 8, 2018

phil-davis commented Jan 8, 2018

PVince81 commented Jan 8, 2018

phil-davis commented Jan 8, 2018

lock bot commented Aug 1, 2019

CLAassistant commented Oct 28, 2017 •

edited

Loading

codecov bot commented Oct 28, 2017 •

edited

Loading

splitt3r Oct 30, 2017 •

edited

Loading