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 style for password protected shares #38894

Merged
merged 2 commits into from
Jun 28, 2021
Merged

Conversation

AlexAndBear
Copy link

@AlexAndBear AlexAndBear commented Jun 25, 2021

Description

With this PR we remove the arrow button in the password input and replace it with separated proceed button

Related Issue

Screenshots (if appropriate):

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@update-docs
Copy link

update-docs bot commented Jun 25, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@AlexAndBear
Copy link
Author

Updated old changelog

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline webUISharingPublic1-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/30807/149/1

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline webUISharingPublic1-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/30808/149/1

@AlexAndBear
Copy link
Author

TBD fix tests

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline webUISharingPublic1-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/30809/149/1

@AlexAndBear AlexAndBear force-pushed the fix-password-proctected-share-ui branch 2 times, most recently from d92d417 to a5a5f8f Compare June 26, 2021 10:51
@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline apiSharingNotificationsToRoot-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/30811/104/1

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline apiSharingNotificationsToShares-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/30811/105/1

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/30811/105/13

 Scenario: share to user sends notification                                                            # /drone/src/tests/acceptance/features/apiSharingNotificationsToShares/sharingNotifications.feature:25
    Given parameter "shareapi_auto_accept_share" of app "core" has been set to "no"                     # AppConfigurationContext::serverParameterHasBeenSetTo()
    And user "Alice" has uploaded file "filesForUpload/textfile.txt" to "/textfile0.txt"                # FeatureContext::userHasUploadedAFileTo()
    When user "Alice" shares folder "/PARENT" with user "Brian" using the sharing API                   # FeatureContext::userSharesFileWithUserUsingTheSharingApi()
    And user "Alice" shares file "/textfile0.txt" with user "Brian" using the sharing API               # FeatureContext::userSharesFileWithUserUsingTheSharingApi()
    Then user "Brian" should have 2 notifications                                                       # NotificationsCoreContext::userNumNotifications()
    And the last notification of user "Brian" should match these regular expressions about user "Alice" # NotificationsCoreContext::matchNotificationRegularExpression()
      | key         | regex                                            |
      | app         | /^files_sharing$/                                |
      | subject     | /^"%displayname%" shared "PARENT" with you$/     |
      | message     | /^"%displayname%" invited you to view "PARENT"$/ |
      | link        | /^(\/index\.php)?\/f\/(\d+)$/                    |
      | object_type | /^local_share$/                                  |
      '/^(\/index\.php)?\/f\/(\d+)$/' does not match 'https://server/f/2147483661'
      Failed asserting that false is not false.

I suspect that notifications PR owncloud/notifications#342 has changed the link reported in a notification.

Investigating in #38895

@AlexAndBear
Copy link
Author

@phil-davis thanks, yes this is desired, notifications include relative links (only the API will response with (concatenated) absolute link.

@phil-davis
Copy link
Contributor

@janackermann #38896 has been merged.

Please rebase this PR

@AlexAndBear AlexAndBear force-pushed the fix-password-proctected-share-ui branch from a5a5f8f to 93796cd Compare June 26, 2021 14:57
@AlexAndBear
Copy link
Author

@phil-davis thanks 👍 rebased

@AlexAndBear
Copy link
Author

@mmatel Docs relevant ?

@AlexAndBear
Copy link
Author

God damned, now Sonarcloud complains :/

@phil-davis
Copy link
Contributor

I added a commit to exclude all the template files from SonarCloud coverage calculations. IMO there is "no such thing" as unit tests for those.

Maybe the files to be included/excluded for coverage calculations needs to be refined more. But this will hopefully make SonarCloud happy with this PR.

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline apiAuthOcs-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/30818/53/1

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline apiAuth-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/30818/52/1

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline apiCapabilities-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/30818/55/1

@AlexAndBear
Copy link
Author

This PR is cursed 👻

@sonarcloud
Copy link

sonarcloud bot commented Jun 26, 2021

@AlexAndBear
Copy link
Author

@phil-davis still fails :/

@phil-davis
Copy link
Contributor

phil-davis commented Jun 28, 2021

@janackermann is this really still Draft?

@micbar IMO the SonarCloud change here is fine. But we still need to do more to sort out exactly what to do about this sort of PR. It's up to you to decide if you override and merge.

SonarCloud: https://sonarcloud.io/component_measures?id=owncloud_core&metric=new_coverage&pullRequest=38894&selected=owncloud_core%3Aapps%2Ffiles_sharing%2Fjs%2Fauthenticate.js&view=list

It is complaining about authenticate.js not have unit test coverage.

@AlexAndBear AlexAndBear marked this pull request as ready for review June 28, 2021 07:36
@AlexAndBear
Copy link
Author

@phil-davis Converted to ready-to-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[QA] Public link password protection web shows a misplaced arrow in mobile browser
5 participants