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

[FEATURE REQUEST] Make automatic discovery of the account #4325

Merged
merged 4 commits into from
Mar 8, 2024

Conversation

Aitorbp
Copy link
Contributor

@Aitorbp Aitorbp commented Feb 21, 2024

Related Issues

App:#4301

  • Add changelog files for the fixed issues in folder changelog/unreleased. More info here
  • Add feature to Release Notes in ReleaseNotesViewModel.kt creating a new ReleaseNote() with String resources (if required)

QA

Test plan: https://github.com/owncloud/QA/blob/master/Mobile/Android/Executions/Release_4.3/Discovery.md

Some regards: #4325 (comment)

@Aitorbp Aitorbp self-assigned this Feb 21, 2024
@Aitorbp Aitorbp linked an issue Feb 21, 2024 that may be closed by this pull request
@Aitorbp Aitorbp force-pushed the feature/make_automatic_discovery_account branch 2 times, most recently from e38e4db to 5694725 Compare February 22, 2024 08:15
@JuancaG05 JuancaG05 self-requested a review March 5, 2024 15:26
Copy link
Collaborator

@JuancaG05 JuancaG05 left a comment

Choose a reason for hiding this comment

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

Some tiny changes here and ready to go @Aitorbp

@Aitorbp Aitorbp force-pushed the feature/make_automatic_discovery_account branch from 8ee5fed to dc84b4b Compare March 5, 2024 22:37
Copy link
Collaborator

@JuancaG05 JuancaG05 left a comment

Choose a reason for hiding this comment

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

LGTM

@jesmrec
Copy link
Collaborator

jesmrec commented Mar 6, 2024

It seems that the branch is conflicting with the current master, could you take a look?

@Aitorbp Aitorbp force-pushed the feature/make_automatic_discovery_account branch from dc84b4b to fb92f06 Compare March 6, 2024 17:50
@jesmrec
Copy link
Collaborator

jesmrec commented Mar 7, 2024

Feature works fine but is not good-performant. The process to discover is the same as before (so, correct) but the complete discovery in accounts with great amount of folders is pretty slow. Just an example

Account with:

~13.000 folders
+20.000 files

took longer than 3 hours to be completely discovered. As first approach is OK, because the given example is just an extreme one to test the app. But, probably, there are other ways to do this task in a more optimal way like depth infinity (needs server logic as well).

Other feature (not a bug): If server connection is lost somehow during the discovery process, this is never completed and there is no way to retry/trigger it because of the removal of the circled arrow icon. Known restriction not to forget.

Finally we have a kind of feature-loss. Before, users could sync the account whenever they wanted, now, only posible in first login. That is a known restriction as well.

@jesmrec
Copy link
Collaborator

jesmrec commented Mar 7, 2024

Let's move this forward, without forgetting the known restrictions

@Aitorbp Aitorbp force-pushed the feature/make_automatic_discovery_account branch from fb92f06 to 03906fb Compare March 8, 2024 12:05
@Aitorbp Aitorbp merged commit fa1b2a9 into master Mar 8, 2024
5 checks passed
@Aitorbp Aitorbp deleted the feature/make_automatic_discovery_account branch March 8, 2024 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] Make automatic discovery of the account
3 participants