Conversation
vue migration
usselite
force-pushed
the
nc33_vue_migration
branch
from
55f5db3 to
ce32f70
Compare
Update license headers Set attributes auth new format
- actions/checkout v4 → v6 - actions/setup-node v4 → v6 - node 20 → 22 (current LTS, node 20 EOL April 2026) - composer update → composer install (reproducible builds)
A single composer.lock cannot satisfy PHP 8.0-8.4 simultaneously. Each matrix combination needs to resolve its own compatible deps. composer install remains in static.yml and composer-php.yml where a single PHP version is used and reproducibility matters.
Align with NC33 minimum PHP requirement.
Resolves php-cs-fixer warning and documents supported PHP range.
mehdimkia
reviewed
Feb 10, 2026
mehdimkia
reviewed
Feb 10, 2026
mehdimkia
reviewed
Feb 10, 2026
Cleaning up and fixing
Fix securemail
mehdimkia
reviewed
Feb 11, 2026
| * | ||
| */ | ||
| async function onActivate() { | ||
| await licenseStore.activateLicense(groupsStore.selectedGroupId) |
Contributor
There was a problem hiding this comment.
Doesnt it make sense to wrap this in a try and catch block, what happens when the call fails?
Suggested change
| await licenseStore.activateLicense(groupsStore.selectedGroupId) | |
| async function onActivate() { | |
| try { | |
| await licenseStore.activateLicense(groupsStore.selectedGroupId) | |
| // Optional: Show success toast | |
| } catch (e) { | |
| // Show error notification so user knows WHY it failed | |
| console.error(e) | |
| // specific to Nextcloud: | |
| OC.Notification.showError('Failed to activate license.') | |
| } | |
| } |
Reset template
Fix empty license
davidmrc6
reviewed
Feb 17, 2026
Collaborator
davidmrc6
left a comment
davidmrc6
left a comment
There was a problem hiding this comment.
most of the code i've looked at seems fine, but the PR is too big to fully go in depth and check all code. i've played around with the app for a bit on nextcloud and it looks like it works without any issues, but I might've missed something
some other notes:
- changelog should be updated
- I had to do
npm i -D @nextcloud/browserlist-configbecause I was getting a module not found error, might be a missing dependency (could also be an issue on my machine only)
UX/UI related:
- these buttons being the same color as the bg looks a bit weird, some contrast would look better
- this text is not aligned
- for the retention assistant, I think we should add embedded links to the apps we ask the user to download from NC as searching for apps by name on NC is painful. Also, maybe an info box explaining what the Retention Assistant does?
| async function onSelectGroup(gid: string) { | ||
| groupsStore.selectGroup(gid) | ||
| await Promise.all([ | ||
| settingsStore.loadSettings(gid), |
Collaborator
There was a problem hiding this comment.
if the api calls that loadSettings() makes fail, the whole method fails silently
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.