Create text #4915

tobiasKaminsky · 2019-12-02T12:12:39Z

ezaquarii · 2019-12-19T10:24:08Z

src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java

+    }
+
+    private CapabilityBooleanType getBoolean(Cursor cursor, String columnName) {
+        return CapabilityBooleanType.fromValue(cursor.getInt(cursor.getColumnIndex(columnName)));


CapabilityBooleanType.fromValue can return null - is it safe? While here, we could revisit this behaviour and maybe make it non-null?

I created a new issue for this, so we can fix this separately.

ezaquarii · 2019-12-19T10:26:03Z

src/main/java/com/owncloud/android/files/FetchTemplateOperation.java

-                        templateObject.getString("extension")));
+                                                   templateObject.getString("name"),
+                                                   templateObject.optString("preview"),
+                                                   Template.Type.valueOf(templateObject.getString("type")),


What if conversion fails, for example due to a corruption of future data format change?
This can be addressed by providing custom constructor method with defensive behaviour.

Also, I find having a test case for data format conversions helpful.

How would you do this?
Create a constructor that accepts a JSONObject? And do the "getInt", "getString" there?

And what do you mean with "defensive behaviour"? If it fails, do not create the template?

It input is invalid, return sane default, ie. switch default case. Provided it makes sense to continue in such situation...

https://en.m.wikipedia.org/wiki/Defensive_programming

How would you do this?

Something like this:

enum Type { UNKNOWN, ALPHA, BRAVO; Type parse(String type) { switch (type.toLowerCase(Locale.US)) { case "alpha": return ALPHA; case "bravo": return BRAVO; default: return UNKNOWN; } } }

Or maybe use a temporary variable if linter complains about multiple returns.
Same in in Kotlin:

enum class Type { UNKNOWN, ALPHA, BRAVO; fun parse(type: String) = when (type.toLowerCase(Locale.US)) { "alpha" -> ALPHA "bravo" -> BRAVO else -> UNKNOWN } }

Enum.valueOf() will throw if you give it an invalid string:
https://docs.oracle.com/javase/7/docs/api/java/lang/Enum.html#valueOf(java.lang.Class,%20java.lang.String)
causing a hard crash.

ezaquarii · 2019-12-19T10:29:22Z

src/main/java/com/owncloud/android/files/FileMenuFilter.java

+                            List<Integer> toHide,
+                            OCCapability capability
+    ) {
+        if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) {


We could use DeviceInfo here with editorSupported property.

Good point 👍

ezaquarii · 2019-12-19T10:31:45Z

src/main/java/com/owncloud/android/files/FileMenuFilter.java

-                item.setTitle(String.format(openWith, productName));
-            }
+    @Nullable
+    public static Editor getEditor(ContentResolver contentResolver, Account account, String mimeType) {


Can we use User object? toPlatformAccount() can be used to satisfy ArbitraryDataProvider dependency, but this way we'd make FileMenuFilter.getEditor() calls using new API. ArbitraryDataProvider would catch up later.

True, I forget about this all the time :/

ezaquarii · 2019-12-19T10:31:58Z

src/main/java/com/owncloud/android/files/FileMenuFilter.java

-            }
+    @Nullable
+    public static Editor getEditor(ContentResolver contentResolver, Account account, String mimeType) {
+        String json = new ArbitraryDataProvider(contentResolver).getValue(account, ArbitraryDataProvider.DIRECT_EDITING);


Can it be null?

Yes, for example if text app is not installed on server.

I'd consider makeing it explicit by wrapping it in Java8 Optional<Editor> (we have a port in com.nextcloud.client.jav.util), as this is truly optional feature. This way nobody will accidentally trip over null.

We have Optional ported from Java8 stdlib, so it is independent from Android Runtime version and can be used safely.

BTW If you expect it to be non-null, optionals can be unwrapped quickly with Optional.orElseThrow.

ezaquarii · 2019-12-19T10:35:36Z

src/main/java/com/owncloud/android/providers/DocumentsStorageProvider.java

        boolean isLoading = false;
        if (parentFolder.isExpired()) {
-            final ReloadFolderDocumentTask task = new ReloadFolderDocumentTask(parentFolder, result -> {
+            ArbitraryDataProvider arbitraryDataProvider = new ArbitraryDataProvider(getContext().getContentResolver());


Can we inject it via contructor? It's de factor another dependency and we just made it less visible.

Unfortunately not as this can be called directly via Android (e.g. if you open up any app that can read storage access framework.

Hm... Is MainApp started in such case? I'd say it is and Dagger should be enabled by that time.

I think we decided not Dagger-ize this component for some reason - can't remember exactly what it was...

ezaquarii · 2019-12-19T10:38:45Z

src/main/java/com/owncloud/android/ui/activity/EditorWebView.java

+        initLoadingScreen();
+    }
+
+    // TODO really here?


Probably not :)

It is really the right place as TextEditor can use it, but RichDocumentsEditor overwrites it.
But the later one will be removed with EOL of NC19.

Can we put a comment that this code is scheduled for retirement in near future?

This Todo line is a bit scary :)

ezaquarii · 2019-12-19T11:59:12Z

src/main/java/com/owncloud/android/ui/activity/TextEditorWebView.kt

+                appVersion = pInfo.versionName
+            }
+        } catch (e: NameNotFoundException) {
+            Log_OC.e(this, "Trying to get packageName", e.cause)


We have AppInfo and DeviceInfo to provide data about runtime environment. Both appVersion and androidVersion could be moved there.

This way we don't need to do all the error handling in UI and we can inject them in tests. This exception is effectively unhandled, leading to undefined behaviour.

Not that we expect it to fail... We can assume that if it fails, it will be because of a platform bug or some "security features", possibly causing some sort of unexplained "glitch".

I'd consider moving that to AppInfo and throw RuntimeException("Platform bug?", e) if we can't retrieve it.

Done, but I chose not to throw an exception, but return "n/a" as it is not critical to have the exact number.

src/main/java/com/owncloud/android/ui/adapter/RichDocumentsTemplateAdapter.java

src/main/java/com/owncloud/android/ui/dialog/ChooseRichDocumentsTemplateDialogFragment.java

codecov · 2020-01-06T14:10:09Z

Codecov Report

Merging #4915 into master will decrease coverage by 0.16%.
The diff coverage is 0.96%.

@@             Coverage Diff              @@
##             master    #4915      +/-   ##
============================================
- Coverage     17.79%   17.62%   -0.17%     
  Complexity        3        3              
============================================
  Files           389      392       +3     
  Lines         32890    33105     +215     
  Branches       4626     4647      +21     
============================================
- Hits           5852     5835      -17     
- Misses        26082    26315     +233     
+ Partials        956      955       -1

Impacted Files	Coverage Δ	Complexity Δ
...java/com/nextcloud/client/di/ComponentsModule.java	`0% <ø> (ø)`	`0 <0> (ø)`	⬇️
...ain/java/com/owncloud/android/db/ProviderMeta.java	`84.61% <ø> (ø)`	`0 <0> (ø)`	⬇️
...m/owncloud/android/ui/adapter/TemplateAdapter.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...ndroid/ui/asynctasks/RichDocumentsLoadUrlTask.java	`0% <0%> (ø)`	`0 <0> (?)`
...ndroid/ui/dialog/ChooseTemplateDialogFragment.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...droid/ui/fragment/OCFileListBottomSheetDialog.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...owncloud/android/files/FetchTemplateOperation.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...cloud/android/ui/preview/PreviewMediaFragment.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...d/android/ui/asynctasks/TextEditorLoadUrlTask.java	`0% <0%> (ø)`	`0 <0> (?)`
.../owncloud/android/ui/activity/TextEditorWebView.kt	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
... and 24 more

- abstract EditorWebView - support direct editing endpoint Create new files via direct editing Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

nextcloud-android-bot · 2020-01-07T07:37:36Z

Codacy

329

Lint

Type	master	PR
Warnings	74	74
Errors	0	0

SpotBugs (new)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	71
Internationalization Warnings	13
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	106
Security Warnings	44
Dodgy code Warnings	139
Total	413

SpotBugs (master)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	71
Internationalization Warnings	13
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	106
Security Warnings	44
Dodgy code Warnings	138
Total	412

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

nextcloud-android-bot · 2020-01-07T11:00:23Z

Here is an overview of what got changed by this pull request:

Issues
======
- Added 1
           

Complexity increasing per file
==============================
- src/main/java/com/owncloud/android/ui/dialog/ChooseRichDocumentsTemplateDialogFragment.java  7
- src/main/java/com/owncloud/android/ui/asynctasks/RichDocumentsLoadUrlTask.java  3
- src/main/java/com/owncloud/android/ui/asynctasks/TextEditorLoadUrlTask.java  4
- src/main/java/com/nextcloud/client/appinfo/AppInfoImpl.java  2
- src/main/java/com/owncloud/android/ui/fragment/OCFileListBottomSheetDialog.java  3
- src/main/java/com/owncloud/android/ui/adapter/RichDocumentsTemplateAdapter.java  4
         

Complexity decreasing per file
==============================
+ src/main/java/com/owncloud/android/ui/activity/RichDocumentsEditorWebView.java  -1
+ src/main/java/com/owncloud/android/ui/adapter/TemplateAdapter.java  -2
+ src/main/java/com/owncloud/android/ui/activity/EditorWebView.java  -4

See the complete overview on Codacy

nextcloud-android-bot · 2020-01-07T11:00:24Z

src/main/java/com/owncloud/android/files/FileMenuFilter.java


    private void filterSync(List<Integer> toShow, List<Integer> toHide, boolean synchronizing) {
-        if (mFiles.isEmpty() || (!anyFileDown() && !containsFolder()) || synchronizing) {
+        if (files.isEmpty() || (!anyFileDown() && !containsFolder()) || synchronizing) {


Issue found: Useless parentheses.

nextcloud-android-bot · 2020-01-07T13:36:24Z

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12148.apk

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

nextcloud-android-bot · 2020-01-07T13:49:39Z

Codacy

329

Lint

Type	master	PR
Warnings	74	74
Errors	0	0

SpotBugs (new)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	71
Internationalization Warnings	13
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	106
Security Warnings	44
Dodgy code Warnings	139
Total	413

SpotBugs (master)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	71
Internationalization Warnings	13
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	106
Security Warnings	44
Dodgy code Warnings	138
Total	412

tobiasKaminsky · 2020-01-08T05:24:07Z

Tested by Julius

8751c85 Merge pull request #5120 from nextcloud/spinner 9d2b80d spinner: if primary color is white, use grey de269b2 Merge pull request #5115 from nextcloud/311bump 3baa9fe Merge pull request #5118 from nextcloud/fastlane ee6e7ce fix fastlane RC 1343ba0 Merge pull request #5114 from nextcloud/richWorkspaceFixes 1ec8b06 - fix not clickable rich workspace area - fix crash on first loading 62aaed7 bump to 3.11 alpha 9a47a4d Merge pull request #5107 from nextcloud/whiteHeader 6cb0f10 Merge pull request #4915 from nextcloud/createText 3011097 [tx-robot] updated from transifex 248fe1a daily dev 20200108

tobiasKaminsky force-pushed the directEditing branch from 5e60212 to a03a6cf Compare December 2, 2019 12:16

tobiasKaminsky requested a review from AndyScherzinger December 3, 2019 11:51

tobiasKaminsky added the 2. developing label Dec 3, 2019

tobiasKaminsky mentioned this pull request Dec 4, 2019

Create new text document #4930

Closed

tobiasKaminsky force-pushed the createText branch from 198c39b to c8aa596 Compare December 16, 2019 12:57

tobiasKaminsky force-pushed the directEditing branch from 028ea47 to 2bc455e Compare December 18, 2019 07:23

tobiasKaminsky force-pushed the createText branch from c8aa596 to e0fa258 Compare December 18, 2019 09:33

tobiasKaminsky changed the title ~~[WIP] Create text~~ Create text Dec 18, 2019

tobiasKaminsky requested a review from ezaquarii December 18, 2019 09:34

tobiasKaminsky added 3. to review and removed 2. developing labels Dec 18, 2019

tobiasKaminsky force-pushed the directEditing branch from 3a2e573 to 0d84a2d Compare December 19, 2019 07:58

ezaquarii reviewed Dec 19, 2019

View reviewed changes

tobiasKaminsky mentioned this pull request Dec 20, 2019

CapabilityBooleanType.fromValue can return null #5045

Closed

tobiasKaminsky changed the base branch from directEditing to master January 6, 2020 09:15

tobiasKaminsky force-pushed the createText branch 2 times, most recently from e51be4b to 1268eb1 Compare January 6, 2020 14:10

tobiasKaminsky force-pushed the createText branch from 93a0712 to 62b6edd Compare January 7, 2020 07:20

tobiasKaminsky added 2 commits January 7, 2020 08:23

Direct editing support

59585d0

- abstract EditorWebView - support direct editing endpoint Create new files via direct editing Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

fixes from CI

74f8a72

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

tobiasKaminsky force-pushed the createText branch from 62b6edd to 74f8a72 Compare January 7, 2020 07:23

tobiasKaminsky added this to the Nextcloud App 3.10.0 milestone Jan 7, 2020

Better handling of parsing enum

9556a69

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

nextcloud-android-bot reviewed Jan 7, 2020

View reviewed changes

tobiasKaminsky merged commit 6cb0f10 into master Jan 8, 2020

delete-merged-branch bot deleted the createText branch January 8, 2020 05:24

joshtrichards mentioned this pull request Oct 12, 2023

Add action to a text file in the current directory #417

Closed

Uh oh!

Create text #4915

Create text #4915

Uh oh!

Conversation

tobiasKaminsky commented Dec 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezaquarii Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nextcloud-android-bot commented Jan 7, 2020

Codacy

Lint

SpotBugs (new)

SpotBugs (master)

Uh oh!

nextcloud-android-bot commented Jan 7, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nextcloud-android-bot commented Jan 7, 2020

Uh oh!

nextcloud-android-bot commented Jan 7, 2020

Codacy

Lint

SpotBugs (new)

SpotBugs (master)

Uh oh!

tobiasKaminsky commented Dec 2, 2019 •

edited

Loading

ezaquarii Dec 20, 2019 •

edited

Loading

tobiasKaminsky Dec 20, 2019 •

edited

Loading

codecov bot commented Jan 6, 2020 •

edited

Loading