[Refactoring] Replacement of models + QML refactored#1293
[Refactoring] Replacement of models + QML refactored#1293sklencar merged 22 commits intoprojects-model-refactoring-mainfrom
Conversation
|
Under My projects page, can we get rid of the namespace (tomas in your video). This is inline with the Mergin website too: |
|
Sure Saber, that is not a problem. Do we want it only in |
|
Correct, as it is clear that it is user's projects. Other pages (Home, Shared with me..) there will be different namespaces and can cause confusion |
| if ( mProjects[i].projectDir == projectDir ) | ||
| if ( mProjects[i].id() == projectId ) | ||
| { | ||
| emit aboutToRemoveLocalProject( mProjects[i] ); |
There was a problem hiding this comment.
I see that this signal is propagated to ProjectsModel. Does it have some advantage to emit it before project is removed from a disk and local manager's project?
There was a problem hiding this comment.
I think generally it is good idea to first remove any reference to the project and then actually remove it from device (in case we would want to check if the dir exists or not). Do not have a preference though
| InputUtils::log( "list projects", QStringLiteral( "Success - got %1 projects" ).arg( mRemoteProjects.count() ) ); | ||
| InputUtils::log( "list projects", QStringLiteral( "Success - got %1 projects" ).arg( projectList.count() ) ); | ||
| } | ||
| else |
There was a problem hiding this comment.
Generally, handling if anything goes wrong with fetching projects from Mergin, was deleted (2 places). The previous behaviours is that the model clears data.
To reproduce:
- fetch shared with me
- log out
- try to go to shared with me -> auth request triggers Login page
- cancel/go back from Login page, you can still see shared with me projects listed
There was a problem hiding this comment.
MerginAPI used to held a separated list of projects which is now gone and after listProjects is finished, it now emits all projects and not care what happens next.
Good idea though, I will connect log out signal to CreatedProjectsModel and SharedProjectsModel types!
| } | ||
| project->mergin->status = ProjectStatus::projectStatus( project ); | ||
| } | ||
| else if ( project->local->localVersion > -1 ) |
There was a problem hiding this comment.
As we discuss, this may introduce confusion about remote projects - if they are from different mergin server or they are orphaned. Maybe would be good to set some specific project status for such projects - so a user can see, that they are Mergin projects, but actually cannot sync as usual with current auth session.
There was a problem hiding this comment.
I think that we actually do not need it anymore thanks to this https://github.com/lutraconsulting/input/blob/pmr-project-sync/app/projectsmodel.cpp#L623
I will double check that
|
@saberraz this is how it looks without namespace. Can be?
|
|
Merging the PR, some of the remarks will be resolved in the next PR |
This PR contains all parts of refactoring (including all points fixed from #1293 ) and Mergin cpp api. Commit containing Mergin cpp api: 13dfde6 All other commits contain refactoring. Commits from 557dfd1 contain UI fixes of #1293 + code cleanup and documentation (I tried to separate them ~ one fix = one commit ~ so it should be possible to review them commit by commit). Versions were also updated to 0.9.3 Resolves #379 Resolves #352 Resolves #1050 Also #1086 (if I understand it correctly, app now shows when project is being uploaded) Co-authored-by: Peter Petrik <zilolv@gmail.com>
4 participants
QML is refactored, we have new separated components. C++ is updated based on that and also:
_futuresuffixCurrent known issues / Todo in this or next PR:
Screen_Recording_20210331-131711.mp4