Added calculation of the project progress on projects list initializa… #1167

mXaln · 2019-07-29T20:26:40Z

…tion

jsarabia · 2019-07-30T13:11:19Z

...wycliffeassociates/translationrecorder/ProjectManager/activities/ActivityProjectManager.java

@@ -199,6 +199,7 @@ private void initializeViews() {

        hideProjectsIfEmpty(mNumProjects);
        if (mNumProjects > 0) {
+            calculateProjectsProgress();


I don't want to put this here; there's already too much work going on to start up this activity that we need to make sure that things that won't change very often don't happen all the time.

progress is only "wrong" when the filesystem and database were out of sync, correct?
Then let's add a resultcode to the resync to return whether or not anything changed, and only on change do we calculate all the project progress.

jsarabia · 2019-07-30T13:20:01Z

...src/main/java/org/wycliffeassociates/translationrecorder/database/ProjectDatabaseHelper.java

+        String projectIdString = String.valueOf(projectId);
+        SQLiteDatabase db = getReadableDatabase();
+        final String query = String.format("SELECT %s FROM %s WHERE %s=?",
+                ProjectContract.ProjectEntry.PROJECT_PROGRESS, ProjectContract.ProjectEntry.TABLE_PROJECT, ProjectContract.ProjectEntry._ID);


when you have to split arguments onto another line because of length (and this still has line length issues), each parameter goes on its own line like so:

final String query = String.format( "SELECT %s FROM %s WHERE %s=?", ProjectContract.ProjectEntry.PROJECT_PROGRESS, ProjectContract.ProjectEntry.TABLE_PROJECT, ProjectContract.ProjectEntry._ID );

jsarabia · 2019-07-30T13:20:20Z

...src/main/java/org/wycliffeassociates/translationrecorder/database/ProjectDatabaseHelper.java

+        SQLiteDatabase db = getReadableDatabase();
+        ContentValues contentValues = new ContentValues();
+        contentValues.put(ProjectContract.ProjectEntry.PROJECT_PROGRESS, progress);
+        db.update(ProjectContract.ProjectEntry.TABLE_PROJECT, contentValues, whereClause, new String[]{projectIdString});


line length

jsarabia · 2019-07-31T14:14:50Z

...a/org/wycliffeassociates/translationrecorder/ProjectManager/activities/ActivityUnitList.java

@@ -192,13 +195,20 @@ private void prepareUnitCardData() {
            List<Chunk> chunks = chunkPlugin.getChapter(mChapterNum).getChunks();
            for (Chunk unit : chunks) {
                String title = Utils.capitalizeFirstLetter(mProject.getModeName()) + " " + unit.getLabel();
-                mUnitCardList.add(new UnitCard(mAdapter, mProject, title, mChapterNum, unit.getStartVerse(), unit.getEndVerse()));
+                mUnitCardList.add(new UnitCard(mAdapter, mProject, title, mChapterNum, unit.getStartVerse(), unit.getEndVerse(), this));


line length

@sarabiaj Updated

jsarabia · 2019-07-31T15:38:47Z

...er/app/src/main/java/org/wycliffeassociates/translationrecorder/project/ProjectProgress.java

+
+    public int calculateProjectProgress() {
+        try {
+            ChunkPlugin chunks = project.getChunkPlugin(new ChunkPluginLoader(context));


why not do this in the constructor, and move it to the top as a member variable since most every method here uses it?

@sarabiaj That's a good idea. Updated!

jsarabia · 2019-07-31T15:40:47Z

...er/app/src/main/java/org/wycliffeassociates/translationrecorder/project/ProjectProgress.java

+        }
+    }
+
+    public Chapter getChapter(int chapterNumber) {


this really shouldn't be a public API for a progress use case object

@sarabiaj Updated!

jsarabia · 2019-07-31T17:52:20Z

...wycliffeassociates/translationrecorder/ProjectManager/activities/ActivityProjectManager.java

-import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.OutputStream;
+import java.io.*;


don't import .*

@sarabiaj yeah, I know... It's just my ide does that, and I didn't pay attention

jsarabia · 2019-07-31T18:24:35Z

...nRecorder/app/src/main/java/org/wycliffeassociates/translationrecorder/widgets/UnitCard.java


    // Constructors
-    public UnitCard(DatabaseAccessor db, Project project, String title, int chapter, int firstVerse, int endVerse) {
+    public UnitCard(DatabaseAccessor db, Project project, String title, int chapter, int firstVerse, int endVerse, OnTakeDeleteListener listener) {


line length

jsarabia · 2019-07-31T18:32:45Z

...er/app/src/main/java/org/wycliffeassociates/translationrecorder/project/ProjectProgress.java

+        return Math.round(((float) current / total) * 100);
+    }
+
+    public void destroy() {


Manual resource cleanup is difficult and should probably be considered an antipattern in a garbage collected language especially.

nulling out these objects isn't necessary either, they will get garbage collected when ProjectProgress gets garbage collected when the Activity is flagged for GC.

Fortunately, because of what we discussed about injecting the database, you wouldn't need to worry about closing (or opening) the database in this class.

@sarabiaj Yep, I've just removed this method

jsarabia · 2019-07-31T18:53:48Z

...ycliffeassociates/translationrecorder/ProjectManager/tasks/resync/ProjectListResyncTask.java

@@ -34,15 +40,16 @@

    Context mCtx;
    FragmentManager mFragmentManager;
+    ProjectDatabaseHelper mDb;

    public ProjectListResyncTask(int taskId, Context ctx, FragmentManager fm) {


Lets Inject the database here too

jsarabia · 2019-07-31T19:15:55Z

...a/org/wycliffeassociates/translationrecorder/ProjectManager/activities/ActivityUnitList.java

        }
    }

+    @Override
+    public void onTakeDeleted() {
+        ProjectProgress pp = new ProjectProgress(mProject, db, chunkPlugin.getChapters());


this can be moved to the top; I'd name it projectProgress over pp though since its use will be further from the declaration. (you're in the unit list, so everything is in the same project, this will cut down on a lot of work being done repeatedly)

jsarabia · 2019-07-31T19:18:46Z

...ycliffeassociates/translationrecorder/ProjectManager/tasks/resync/ProjectListResyncTask.java


-    public ProjectListResyncTask(int taskId, Context ctx, FragmentManager fm) {
+    public ProjectListResyncTask(int taskId, Context ctx, FragmentManager fm, ProjectDatabaseHelper db) {


if you inject the chunkpluginloader, you won't need the context object dependency here

jsarabia · 2019-07-31T19:21:09Z

...ycliffeassociates/translationrecorder/ProjectManager/tasks/resync/ProjectListResyncTask.java

        Map<Project, File> directoriesOnFs = getProjectDirectoriesOnFileSystem();
        //if the number of projects doesn't match up between the filesystem and the db, OR,
        //the projects themselves don't match an id in the db, then resync everything (only resyncing
        // projects missing won't remove dangling take references in the db)
        //NOTE: removing a project only removes dangling takes, not the project itself from the db
-        if (directoriesOnFs.size() != db.getNumProjects() || db.projectsNeedingResync(directoriesOnFs.keySet()).size() > 0) {
-            fullResync(db, directoriesOnFs);
+        if (directoriesOnFs.size() != mDb.getNumProjects() || mDb.projectsNeedingResync(directoriesOnFs.keySet()).size() > 0) {


line length

jsarabia · 2019-07-31T19:48:09Z

...pp/src/main/java/org/wycliffeassociates/translationrecorder/Recording/RecordingActivity.java

+        if(chunkPlugin != null) {
+            ProjectProgress pp = new ProjectProgress(mProject, db, chunkPlugin.getChapters());
+            pp.updateProjectProgress();
+        }
        db.close();


probably not good to close this if we're going to have one instance for the whole activity

jsarabia · 2019-07-31T20:02:38Z

...nRecorder/app/src/main/java/org/wycliffeassociates/translationrecorder/widgets/UnitCard.java

@@ -145,7 +162,11 @@ private void refreshAudioPlayer() {
                ap.loadFile(getTakeList().get(mTakeIndex));
            }
        }
-        ap.refreshView(mViewHolder.elapsed, mViewHolder.duration, mViewHolder.takePlayPauseBtn, mViewHolder.seekBar);
+        ap.refreshView(mViewHolder.elapsed,


first parameter goes on a new line like you did on line 135

jsarabia · 2019-08-02T13:44:21Z

...a/org/wycliffeassociates/translationrecorder/ProjectManager/activities/ActivityUnitList.java

            List<Chunk> chunks = chunkPlugin.getChapter(mChapterNum).getChunks();
            for (Chunk unit : chunks) {
                String title = Utils.capitalizeFirstLetter(mProject.getModeName()) + " " + unit.getLabel();
-                mUnitCardList.add(new UnitCard(mAdapter, mProject, title, mChapterNum, unit.getStartVerse(), unit.getEndVerse()));
+                mUnitCardList.add(new UnitCard(


code formatting

jsarabia · 2019-08-02T13:44:54Z

...liffeassociates/translationrecorder/recordingapp/UnitTests/project/ProjectFileUtilsTest.java


    String jarName = "biblechunk.jar";
    String className = "org.wycliffeassociates.translationrecorder.biblechunk.BibleChunkPlugin";
-    Anthology anthology = new Anthology("ot", "Old Testament", "bible", regex, groups, mask, jarName, className);
+    Anthology anthology = new Anthology("ot", "Old Testament", "bible", sort, regex, groups, mask, jarName, className);


line length

jsarabia · 2019-08-02T13:47:06Z

...a/org/wycliffeassociates/translationrecorder/FilesPage/Export/TranslationExchangeExport.java

@@ -54,7 +54,8 @@ protected void handleUserInput() {
            @Override
            public void run() {
                Context ctx = mCtx.getActivity().getApplicationContext();
-                uploadBinary(ctx, outputFile());
+                ProjectDatabaseHelper db = ((TranslationRecorderApp)mCtx.getActivity().getApplication()).getDatabase();


don't do this; provide it as a dependency

jsarabia · 2019-08-02T13:48:23Z

translationRecorder/app/src/main/java/org/wycliffeassociates/translationrecorder/MainMenu.java

@@ -287,6 +284,8 @@ private void initApp() {
            brd.show(fm, "Bug Report Dialog");
        }

+        db = ((TranslationRecorderApp)getApplication()).getDatabase();


this is not related to initializing views; maybe move it to onCreate

jsarabia · 2019-08-02T13:50:15Z

translationRecorder/app/src/main/AndroidManifest.xml

@@ -96,6 +96,15 @@
        <service android:name="org.wycliffeassociates.translationrecorder.Recording.WavRecorder" />
        <service android:name="org.wycliffeassociates.translationrecorder.Recording.WavFileWriter" />

+        <provider


what is this needed for?

@sarabiaj Since Android api 24, in order for the app to give an access to it's content to other apps, this FileProvider is needed. You can try to share a project on devices with Android 7 and up, it will crash.

https://developer.android.com/training/secure-file-sharing/setup-sharing

jsarabia · 2019-08-02T13:53:59Z

...a/org/wycliffeassociates/translationrecorder/ProjectManager/adapters/ChapterCardAdapter.java

+            AppCompatActivity context,
+            Project project,
+            List<ChapterCard> chapterCardList,
+            ProjectDatabaseHelper db) {


put the ) { on a new line

jsarabia · 2019-08-02T13:54:28Z

.../java/org/wycliffeassociates/translationrecorder/ProjectManager/adapters/ProjectAdapter.java

@@ -71,12 +68,12 @@ public View getView(final int position, View convertView, final ViewGroup parent
            holder = (ViewHolder) convertView.getTag();
        }

-        initializeProjectCard(mCtx, mProjectList.get(position), mDb, holder.mLanguage, holder.mBook, holder.mInfo, holder.mRecord, holder.mTextLayout, holder.mProgressPie);
+        initializeProjectCard(mCtx, mProjectList.get(position), db, holder.mLanguage, holder.mBook, holder.mInfo, holder.mRecord, holder.mTextLayout, holder.mProgressPie);


line length

jsarabia · 2019-08-02T13:54:38Z

.../java/org/wycliffeassociates/translationrecorder/ProjectManager/adapters/ProjectAdapter.java


        return convertView;
    }

-    public static void initializeProjectCard(final Activity ctx, final Project project, ProjectDatabaseHelper dB, TextView languageView, TextView bookView,
+    public static void initializeProjectCard(final Activity ctx, final Project project, final ProjectDatabaseHelper dB, TextView languageView, TextView bookView,


line length

jsarabia · 2019-08-02T13:55:15Z

...java/org/wycliffeassociates/translationrecorder/ProjectManager/adapters/UnitCardAdapter.java

+            Project project,
+            int chapter,
+            List<UnitCard> unitCardList,
+            ProjectDatabaseHelper db) {


newline for ) {

jsarabia · 2019-08-02T13:56:06Z

...g/wycliffeassociates/translationrecorder/ProjectManager/tasks/resync/DatabaseResyncTask.java

@@ -23,13 +22,13 @@
 * Created by sarabiaj on 9/27/2016.
 */
 public class DatabaseResyncTask extends Task implements ProjectDatabaseHelper.OnLanguageNotFound, ProjectDatabaseHelper.OnCorruptFile {


line length

jsarabia · 2019-08-02T13:56:35Z

...ycliffeassociates/translationrecorder/ProjectManager/tasks/resync/ProjectListResyncTask.java

+                                                        mode = db.getMode(db.getModeId(
+                                                                wav.getMetadata().getModeSlug(),
+                                                                wav.getMetadata().getAnthology()
+                                                        ));


code formatting

jsarabia · 2019-08-02T14:08:04Z

...ycliffeassociates/translationrecorder/ProjectManager/tasks/resync/ProjectListResyncTask.java

-            fullResync(db, directoriesOnFs);
+        if (directoriesOnFs.size() != db.getNumProjects() || db.projectsNeedingResync(
+                directoriesOnFs.keySet()
+        ).size() > 0) {


I know you're meaning to break apart the method call in projects needing resync, but you want to make sure that it lines up vertically when you do that, especially with its beginning and the close parenthesis

if ( directoriesOnFs.size() != db.getNumProjects() || db.projectsNeedingResync(directoriesOnFs.keySet()).size() > 0 ) {

is probably what you would do here? but realistically, you'd rather assign variables

boolean projectCountDiffers = directoriesOnFs.size != db.getNumProjects; boolean projectsNeedResync = db.projectsNeedingResync(directoriesOnFs.keySet()).size > 0 if(projectCountDiffers || projectsNeedResync) {

added benefit here is the if statement is more readable as to its intent

jsarabia · 2019-08-02T14:12:48Z

...rder/app/src/main/java/org/wycliffeassociates/translationrecorder/project/ProjectPlugin.java

+                books.toArray(new Book[books.size()]),
+                versions.toArray(new Version[versions.size()]),
+                modes.toArray(new Mode[modes.size()]),
+                db);


); on new line

Added calculation of the project progress on projects list initializa…

49512a4

…tion

jsarabia reviewed Jul 30, 2019

View reviewed changes

Refactored project/chapter progress computation

a4f5b2d

jsarabia reviewed Jul 31, 2019

View reviewed changes

Long line to break

930e4b9

jsarabia reviewed Jul 31, 2019

View reviewed changes

ProjectProgress: chunkPlugin initialization moved to constructor

ed3e69c

jsarabia reviewed Jul 31, 2019

View reviewed changes

mXaln added 2 commits July 31, 2019 14:57

ProjectProgress: refactored

a8cbdfd

ProjectDatabaseHelper is injected in some classes

11d05fc

jsarabia reviewed Jul 31, 2019

View reviewed changes

mXaln added 2 commits August 1, 2019 15:04

Major refactor: moved database initialization to the app level

b5a9362

fixed some tests

241a32b

jsarabia reviewed Aug 2, 2019

View reviewed changes

Code cleaning

ebf122c

mXaln merged commit c70f811 into export-with-user Aug 6, 2019

mXaln deleted the mm_projects_progress_update branch August 16, 2019 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added calculation of the project progress on projects list initializa… #1167

Added calculation of the project progress on projects list initializa… #1167

mXaln commented Jul 29, 2019

jsarabia Jul 30, 2019

jsarabia Jul 30, 2019

jsarabia Jul 30, 2019 •

edited

Loading

jsarabia Jul 31, 2019

mXaln Jul 31, 2019

jsarabia Jul 31, 2019

mXaln Jul 31, 2019

jsarabia Jul 31, 2019

mXaln Jul 31, 2019

jsarabia Jul 31, 2019

mXaln Jul 31, 2019

jsarabia Jul 31, 2019

jsarabia Jul 31, 2019

mXaln Jul 31, 2019

jsarabia Jul 31, 2019

jsarabia Jul 31, 2019

jsarabia Jul 31, 2019

jsarabia Jul 31, 2019

jsarabia Jul 31, 2019

jsarabia Jul 31, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019

mXaln Aug 2, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019

jsarabia Aug 2, 2019


		public ProjectListResyncTask(int taskId, Context ctx, FragmentManager fm) {
		public ProjectListResyncTask(int taskId, Context ctx, FragmentManager fm, ProjectDatabaseHelper db) {

Added calculation of the project progress on projects list initializa… #1167

Added calculation of the project progress on projects list initializa… #1167

Conversation

mXaln commented Jul 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsarabia Jul 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsarabia Jul 30, 2019 •

edited

Loading