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.

Sign up for GitHub

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

Jump to bottom

[full-ci] feat: move expiration date to a drop #5806

Merged
merged 67 commits into from
Oct 15, 2021
Merged

[full-ci] feat: move expiration date to a drop #5806

merged 67 commits into from
Oct 15, 2021

Conversation

LukasHirt
Copy link
Collaborator

@LukasHirt LukasHirt commented Sep 14, 2021 •
edited by lookacat
Loading

Open tasks:

  • Add changelog
  • Move v-calendar to ODS
  • Release & update ODS
  • Fix tests
  • Write unit tests
  • Discuss clear button

@LukasHirt LukasHirt added the Category:Enhancement Add new functionality label Sep 14, 2021
@LukasHirt LukasHirt self-assigned this Sep 14, 2021
@update-docs

This comment has been minimized.

Sign in to view
@ownclouders

This comment has been minimized.

Sign in to view
@LukasHirt LukasHirt changed the title feat: move expiration date to a drop [full-ci] feat: move expiration date to a drop Sep 15, 2021
@ownclouders

This comment has been minimized.

Sign in to view
@LukasHirt LukasHirt force-pushed the feat/expiration-date-drop branch from 114f715 to ad49927 Compare September 19, 2021 15:22
@ownclouders

This comment has been minimized.

Sign in to view
@ownclouders
Copy link
Contributor

Results for oC10SharingInternalGroups https://drone.owncloud.com/owncloud/web/19144/22/1
The following scenarios passed on retry:

  • webUISharingInternalGroups/shareWithGroups.feature:255

@ownclouders
Copy link
Contributor

Results for oC10SharingInternalGroupsSharingIndicator https://drone.owncloud.com/owncloud/web/19182/24/1
The following scenarios passed on retry:

  • webUISharingInternalGroupsToRootSharingIndicator/shareWithGroups.feature:104

@LukasHirt LukasHirt force-pushed the feat/expiration-date-drop branch from 85e6b4e to 5d2512e Compare September 23, 2021 11:48
@ownclouders
Copy link
Contributor

Results for oCISRename https://drone.owncloud.com/owncloud/web/19218/48/1
The following scenarios passed on retry:

  • webUIRenameFolders/renameFolders.feature:79

@ownclouders
Copy link
Contributor

Results for oC10SharingInternalUsersSharingIndicator https://drone.owncloud.com/owncloud/web/19218/27/1
The following scenarios passed on retry:

  • webUISharingInternalUsersSharingIndicator/shareWithUsers.feature:154
  • webUISharingInternalUsersToRootSharingIndicator/shareWithUsers.feature:98

@ownclouders
Copy link
Contributor

Results for oC10Files1 https://drone.owncloud.com/owncloud/web/19218/12/1
The following scenarios passed on retry:

  • webUIFilesActionMenu/versions.feature:36

@lookacat
Copy link
Contributor

sure

@ownclouders
Copy link
Contributor

Results for oC10Files1 https://drone.owncloud.com/owncloud/web/19592/12/1
The following scenarios passed on retry:

  • webUIFilesActionMenu/versions.feature:36

@fschade fschade mentioned this pull request Oct 13, 2021
Closed
@lookacat lookacat force-pushed the feat/expiration-date-drop branch from 518ff85 to 5e93728 Compare October 13, 2021 13:21
@ownclouders
Copy link
Contributor

Results for oC10Files1 https://drone.owncloud.com/owncloud/web/19624/12/1
The following scenarios passed on retry:

  • webUIFilesActionMenu/versions.feature:15

@lookacat
Copy link
Contributor

5qabyj

pascalwengerter
pascalwengerter reviewed Oct 13, 2021
View reviewed changes
Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

This PR in general looks good to me, but is a good example of why we should move into smaller PRs for better review-ability - this one could have been divided into OCS update, unit test refactoring, share-date related simplification and then the feature of replacing the Datepicker component itself (just as a suggestion)

Also, apart from the smaller comments I added and which I wouldn't consider show-stoppers, I'm not quite sure how to feel about deleting all those acceptance tests that check the share expiry settings by OC10 admins

packages/web-app-files/src/components/SideBar/Shares/Collaborators/CollaboratorsEditOptions.vue Show resolved Hide resolved
packages/web-app-files/src/components/SideBar/Shares/Collaborators/CollaboratorsEditOptions.vue Show resolved Hide resolved
packages/web-app-files/src/components/SideBar/Shares/Collaborators/CollaboratorsEditOptions.vue Show resolved Hide resolved
packages/web-app-files/src/components/SideBar/Shares/Collaborators/EditCollaborator.vue Show resolved Hide resolved
packages/web-app-files/src/components/SideBar/Shares/Collaborators/EditCollaborator.vue Show resolved Hide resolved
packages/web-app-files/src/components/SideBar/Shares/Collaborators/NewCollaborator.vue Outdated Show resolved Hide resolved
packages/web-app-files/src/components/SideBar/Shares/Collaborators/NewCollaborator.vue Show resolved Hide resolved
packages/web-app-files/tests/integration/specs/peopleExpirationDate.spec.js Show resolved Hide resolved
@ownclouders
Copy link
Contributor

Results for oC10Sharing1 https://drone.owncloud.com/owncloud/web/19630/16/1
The following scenarios passed on retry:

  • webUISharingExpirationDate/shareWithExpirationDate.feature:14

@ownclouders
Copy link
Contributor

Results for oC10Files1 https://drone.owncloud.com/owncloud/web/19630/12/1
The following scenarios passed on retry:

  • webUIFilesActionMenu/versions.feature:36

@lookacat lookacat mentioned this pull request Oct 14, 2021
Closed
kulmann
kulmann requested changes Oct 14, 2021
View reviewed changes
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

One major thing: could you adapt the expiration date handling in links (create/edit public links) so that it looks the same as for user shares? Ideally by reusing the same component... if too much work we can keep it like it is.

package.json Outdated Show resolved Hide resolved
packages/web-runtime/package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
packages/web-runtime/package.json Outdated Show resolved Hide resolved
tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js Outdated Show resolved Hide resolved
tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js Outdated
(result) => {
expirationDate = result.value
const date = new Date(result.value)
const dateString =
Copy link
Member

Choose a reason for hiding this comment

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

Can't you use string formatting on the date to achieve the same result? Probably makes sense to just append the time ( 00:00:00), but for the date it looks weirdly handcrafted / hacky.

Copy link
Contributor

@lookacat lookacat Oct 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

its actually copied from calculateDate in sharingHelper so the dates actually match but i can't use that function sadly, but i don't get why the calculateDate has to be this weird

packages/web-app-files/src/components/SideBar/Shares/Collaborators/CollaboratorsEditOptions.vue Show resolved Hide resolved
packages/web-app-files/src/components/SideBar/Shares/Collaborators/EditCollaborator.vue Show resolved Hide resolved
packages/web-app-files/src/components/SideBar/Shares/Collaborators/NewCollaborator.vue Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Oct 14, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

47.8% 47.8% Coverage
0.0% 0.0% Duplication

@lookacat lookacat merged commit c3a6915 into master Oct 15, 2021
@delete-merged-branch delete-merged-branch bot deleted the feat/expiration-date-drop branch October 15, 2021 12:06
@pascalwengerter pascalwengerter mentioned this pull request Oct 15, 2021
Merged
7 tasks
@JammingBen JammingBen mentioned this pull request Dec 10, 2021
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lookacat lookacat lookacat left review comments

@pascalwengerter pascalwengerter pascalwengerter approved these changes

@kulmann kulmann kulmann approved these changes

@fschade fschade Awaiting requested review from fschade

Assignees

@pascalwengerter pascalwengerter

Labels
Category:Enhancement Add new functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@LukasHirt @ownclouders @pascalwengerter @lookacat @kulmann @fschade