suvanl / fixmylinks Goto Github PK

App and system information

- FixMyLinks app version: 1.1.0
- Android version: 14
- Android build number: UP1A.231105.003
- Device model: Google Pixel 6 Pro
- Device type: Physical

Summary of expected behaviour

Sharing a URL (via the FixMyLinks "Share to..." option in the native share sheet) that has a "www" subdomain and doesn't have any URL parameters either:

• applies a matching rule if there is one
  or
• simply retains the original URL if no rule matches this link (default behaviour of any content shared via this share sheet option).

Summary of actual behaviour

Sharing a URL that has a "www" subdomain and doesn't have any URL parameters causes the app to crash with an NPE.

Description of what you think should be changed

The app shouldn't crash when sharing such a link; this is not the expected behaviour. The issue causing an NPE to be thrown should be fixed.

Steps to reproduce the bug

• Share a link such as https://www.framer.com/motion/guide-reduce-bundle-size/ via the native share sheet using the FixMyLinks "Share to..." option
• Observe that the app crashes instead of redrawing the share sheet

Additional info

No response

Summary of the feature

Display a linear progress bar to convey how much the user has progressed in the "add new rule" flow (currently FmlScreen.SelectRuleType and FmlScreen.AddRule).

Rationale behind the feature request

To improve UX by allowing users (particularly first-time users) to understand how long this flow takes to complete, and how much progress they have currently made.

Describe a possible solution

Share the progress state (e.g., a value between 0F and 1F) across screens and use this to set the level of progress on the progress bar.

Definition of done

• The progress bar is visible regardless of layout size class
• Progress bar completion state updates as user progresses through screens
  • Could have:
    • completing form fields also furthers the progress level
    • if the form field is emptied, progress gets rolled back.
• All related tests pass

Additional information

No response

Summary of the feature

Make it possible to disable built-in rules at an individual level and re-enable any disabled ones at any time.

Rationale behind the feature request

It currently isn't possible to disable built-in rules. They are all enabled by default. It should be possible to disable these rules individually as desired and re-enable them at any time.

Describe a possible solution

• Persistently store the enabled state (isEnabled) of the set of built-in rules.

  • Could use (Proto?) DataStore.
  • Alternatively, could add a isBuiltIn / isCustom column to the BaseRule schema and store the built-in rules in the DB, then reusing custom rule enabling/disabling logic would theoretically be seamless or will require little effort to get working.
    • The only potential issue with this is that users will be able to wipe all built-in rule data by clearing the app's storage.
    • Perhaps this can be mitigated by prepopulating the database? Meaning, if the app's storage is cleared, the built-in rules will be re-inserted into the DB once the app is reopened.

• Enable the "Apply this rule" switch on RuleDetailsScreen for built-in rules (currently only enabled for custom rules).

  • Toggling this switch will update the persisted isEnabled value for the currently selected custom rule.

Definition of done

• Each built-in rule's enabled state should be persistently stored on-device.
• There should be no difference in the UX of enabling/disabling custom rules and enabling/disabling built-in rules.
• All related tests should pass.

Additional information

No response

Summary of the feature

This codebase currently uses manual dependency injection.

It may be a good idea to move to automated DI using Dagger/Hilt for the reasons listed in the following section.

Automated DI will not introduce any behavioural changes to the app, but may improve developer experience (DX) and eradicate the issues that manual DI has the potential to introduce (also described in the following section).

Rationale behind the feature request

As the app grows in size, more and more boilerplate code (such as factories) will have to be written, and dependency injection (DI) will get more complex since the scope and lifecycle of containers have to be managed manually, optimising and discarding containers that are no longer needed. This level of complexity and manual memory management ends up making the app more error-prone since there are more areas in which something could be incorrectly implemented without realising further down the line, and since the bugs/memory leaks introduced by these incorrect implementations may be subtle and difficult to debug.

Therefore, the recommended approach is to use automated DI with Hilt (built on top of Dagger), according to the Android developer documentation.

Describe a possible solution

Use Dagger/Hilt to replace manual dependency injection code.

Definition of done

• All manual DI code is replaced with automated DI tools (i.e., Dagger/Hilt).
• Implementing automated DI using Dagger/Hilt does not make the app any less testable or maintainable.
  • Testing all components of the app should not be made any more difficult whatsoever.
• All existing tests pass.

Additional information

Useful resources:

• https://developer.android.com/training/dependency-injection
• https://developer.android.com/training/dependency-injection/dagger-basics
• https://developer.android.com/training/dependency-injection/hilt-android

Summary of the feature

In the UI, display a visual indication of whether a rule is enabled or disabled.

Rationale behind the feature request

Currently, the only way to know if a rule is enabled or disabled is to go to the rule's "edit" screen and check whether the "Enabled" option switch is on or off.

Describe a possible solution

• Display an enabled/disabled indicator in RulesListItem
  • E.g., a checkmark with "Enabled" or a cross with "Disabled" next to it.
  • The text ("Enabled"/"Disabled") animates out after a few seconds to reduce excessive text.
• Display an enabled/disabled indicator on RuleDetailsScreen

Definition of done

• Enabled/disabled status is displayed on RulesListItem
• Enabled/disabled status is displayed somewhere on RuleDetailsScreen
• All related UI tests pass

Additional information

No response

App and system information

- FixMyLinks app version: 1.2.1
- Android version: 14
- Android build number: UE1A.230829.019
- Device model: Generic AVD ("Medium Phone")
- Device type: Emulated

Summary of expected behaviour

An option should exist to delete all rules while on RulesScreen. This option (via a button) used to be there before the UI was changed to use cards to represent rule items, but now no longer exists.

Summary of actual behaviour

No such option exists anymore.

Description of what you think should be changed

A way to achieve this behaviour should be restored, such as by making it possible to select rule items and delete the selected rules, with a "select all" feature too, to restore the aforementioned behaviour.

Steps to reproduce the bug

1. Navigate to RulesScreen
2. If no rules have been saved yet, create at least one
3. Observe that the "Delete all" button is no longer displayed

Additional info

No response

Summary of the feature

Display built-in rule summary info on RulesScreen and HomeScreen in appropriate formats while conveying whether each built-in rule is active or inactive (note that it currently isn't possible to disable built-in rules, but will be in the future).

Rationale behind the feature request

Without reading the documentation, it isn't possible to know what the built-in rules are, and which ones are active or inactive.

Describe a possible solution

RulesScreen

• Have two tabs: "Custom" and "Built-in".
• Custom rules will be displayed under the "Custom" tab, and built-in rules will be displayed under the "Built-in" tab.
• Active/inactive rules will be under separate headings.

HomeScreen

• Have a heading ("Built-in rules") that contains a horizontally scrollable list of square-shaped cards representing each built-in rule, where each card has an icon (i.e., the favicon of the triggerDomain) and an indicator in the top right corner to represent its enabled/disabled state (small green circle for active, small grey (MD outline colour) circle for inactive).

Definition of done

• Built-in rules are displayed on RulesScreen under "Active rules" or "Inactive rules" heading sections
• Built-in rules are displayed on HomeScreen with active/inactive indicator
• All relevant UI tests pass

Additional information

No response

Summary of the feature

Appends the build date (UTC), commit hash, and branch name to the app's versionName, in a format such as {versionName}.YYMMDD-{commit_hash}@{branch_name}.

Rationale behind the feature request

Make identifying between different builds within the same versionName easier by appending the short form of the latest commit hash as well as the current timestamp at build time.

Describe a possible solution

At build time:

• Get the commit hash of the latest commit (perhaps by executing git rev-parse --short HEAD)
• Get the current system time as a Unix timestamp
• Append these values to the versionName in the format {versionName}.YYMMDD-{commit_hash}@{branch_name}

Additional information

No response

App and system information

- FixMyLinks app version: 1.1.0
- Android version: 10 (API 29)
- Android build number: QSR1.190920.001
- Device model: Generic AVD ("Medium Phone")
- Device type: Emulated

Summary of expected behaviour

The 'edit' FAB should be fully visible and should not be obstructed, hidden, or drawn over by any other elements.

Summary of actual behaviour

This FAB is partially hidden by the navigation bar when using 3-button or 2-button navigation, as seen in the attached screenshots (see Additional info section).

Description of what you think should be changed

The FAB should have sufficient padding to ensure that no part of it is hidden behind the system navigation bar.

Steps to reproduce the bug

• Use the app in portrait orientation on a mobile device (so that it has a window width size class of 'Compact')
• Set the system navigation mode to 3-button or 2-button navigation in system settings
• If no rules have been saved, add a new rule
• Tap the 'Rules' destination in the app's bottom navigation bar
• Tap 'View this' on any rule
• Observe that the 'edit' FAB (with the pencil icon) is partially hidden behind the system navigation bar

Additional info

Reference images

2-button navigation:

3-button navigation:

Summary of the feature

Support the wildcard character (*) in the triggerDomain of a rule.

The expected behaviour is that this would match the domain to any subdomain, e.g.: *.youtube.com, so that both m.youtube.com and www.youtube.com will trigger the rule. Should also work for the apex domain (i.e., youtube.com).

Rationale behind the feature request

Prevents duplicate rules from being needed for different subdomains.

For example, a rule with a trigger of youtube.com won't be triggered for m.youtube.com (it will only be triggered on youtube.com and www.youtube.com).

Describe a possible solution

• If the wildcard operator is used, compare the triggerDomain and the given domain without any subdomains
  • i.e., remove the first dot (.) in the URI.host String and everything before it if the URI matches the format of a URL with a subdomain.

Definition of done

• The wildcard operator gets processed and this rule gets triggered regardless of the subdomain in the URL extracted from the Intent content (if any)

Additional information

No response

App and system information

- FixMyLinks app version: 1.2.1 (ui-refine branch)
- Android version: 13
- Android build number: TE1A.220922.010
- Device model: Pixel 7 Pro
- Device type: Emulated

Summary of expected behaviour

The Box layout composable with a custom background shape of ScallopShape retains its background in place after being pushed down when the SearchBar composable's 'active' state becomes true and goes back to false.

Summary of actual behaviour

The custom ScallopShape background shape of this Box gets lost, resulting in the Box having no background. Its content (in this case an Icon) still gets displayed as expected.

Description of what you think should be changed

The actual behaviour is incorrect as the background should remain in place; there is no reason for it to be lost.

This should be fixed by ensuring that the background remains in place when the Box is pushed back up after closing the search bar.

Steps to reproduce the bug

1. Navigate to RulesScreen.
2. If you already have any rules added, delete all of them.
3. See the 'No rules added' text with a graphic above it, which is a Box containing an Icon as its content. The Box has a custom background shape of ScallopShape.
4. Open the search bar at the top of the screen.
5. Close the search bar.
6. Notice how the background of the Box containing the Icon has disappeared.

Additional info

Screen recording:

Summary of the feature

Support saving custom link rules locally in persistent storage and applying them just like built-in rules.

Users should be able to save rules of any supported type, i.e.:

• DOMAIN_NAME
• URL_PARAMS_ALL
• URL_PARAMS_SPECIFIC
• DOMAIN_NAME_AND_URL_PARAMS_ALL

DOMAIN_NAME_AND_URL_PARAMS_SPECIFIC will be user-selectable in the future.

Rationale behind the feature request

Implements a core functionality of the app currently labelled as "coming soon" in the README.

Describe a possible solution

Local data storage

• Set up a Room database along with its entities and data access objects (DAOs) to store custom rules in an on-device SQLite database
• The entities should map to associated domain models which will represent the data outside of the data layer.
• The data will be stored in a polymorphic manner (with joined table inheritance (JTI) - see joined table inheritance vs single table inheritance for more info)
  • There will be a "base_rule" table for fields that are common to all rule types
    • This table will have a "mutation_type" discriminator column
  • Each specific rule type will be represented by a separate table containing fields that are specific to that rule type
  • There will be a 1-to-1 relationship between the "base_rule" table and each specific rule type table.
    • I.e., each specific rule can only be associated with one base rule (and vice versa).

The following ERD depicts a logical model of the local database design.

Applying the rules

• Modify the MutateUriUseCase to take in a list of custom rules to make sure we check whether the extracted URL is the triggerDomain of any custom rules.
  • These rules should be passed in as domain models inheriting from the base mutation model
    • So polymorphism will allow us to easily accept domain models relating to any rule type, then we can use a when...is block to check the specific type of each model and perform logic as required.
• Allow users to update/delete custom rules.

Additional information

Eventually, users will be presented with an option to store these rules remotely (as well as locally) - essentially acting as a cloud backup.

App and system information

- FixMyLinks app version: 1.1.0
- Android version: 14
- Android build number: UP1A.231105.003
- Device model: Google Pixel 6 Pro
- Device type: Physical

Summary of expected behaviour

Applying a rule on a URL that has a "www" subdomain should result in this subdomain being kept in place after applying the URL_PARAMS_ALL rule.
For example, for a URL_PARAMS_ALL rule on "youtube.com":

• Before: https://www.youtube.com/watch?v=video_id_here
• After: https://www.youtube.com/watch

Summary of actual behaviour

After applying a rule on a link that has a "www" subdomain (such as https://www.youtube.com/watch?v=video_id_here), the modified link produced after applying the rule (such as a URL_PARAMS_ALL rule in this case) gets rid of the "www" subdomain (resulting in https://youtube.com/watch in this case).

In summary:

• Before: https://www.youtube.com/watch?v=video_id_here
• After: https://youtube.com/watch

Description of what you think should be changed

The "www" subdomain getting dropped is a potential issue if the same site isn't served on the apex domain and "www" subdomain.
This should be fixed by ensuring this subdomain gets restored after applying the rule (since, as an implementation detail, the "www" subdomain gets stripped out of the URL if it contains it, before applying the rule logic).

Steps to reproduce the bug

1. Add a new URL_PARAMS_ALL rule
   a. The domain in the "Initial domain" field doesn't have to contain "www"
2. Share a link matching the initial domain of this rule but with a "www" subdomain via the FixMyLinks "Share to..." option in the native share sheet
3. Notice that after the link gets modified, the "www" subdomain is no longer present.

Additional info

No response

App and system information

- FixMyLinks app version: 1.2.1
- Android version: 14 (API 34)
- Android build number: UE1A.230829.019
- Device model: Generic AVD (Medium Phone API 34)
- Device type: Emulated

Summary of expected behaviour

There should be no way to delete a built-in rule.

Summary of actual behaviour

It's possible to "delete" a built-in rule in the same way that a custom rule can be deleted, via RuleDetailsScreen.

The rule doesn't actually get deleted, but there's nothing to indicate that a built-in rule cannot be deleted, so users may expect the rule to have been deleted successfully.

Description of what you think should be changed

The overflow menu containing the "Delete" option on RuleDetailsScreen should only be displayed for custom rules, not built-in rules.

Steps to reproduce the bug

1. Open the app
2. Go to the "Rules" destination using the bottom navigation bar
3. Tap the "Built-in" tab title, or swipe to it
4. Tap on any built-in rule card
5. Activate the overflow menu in the top right of the screen
6. Tap "Delete"
7. Tap "Delete" again on the confirmation dialog
8. Observe that the app navigates back to RulesScreen, just like when a custom rule is deleted.
   • Unlike custom rules, the built-in rule doesn't actually get deleted. The rule not being deleted is a good thing, because it shouldn't be possible to delete built-in rules (and at this moment in time isn't technically possible, given that BIRs are hardcoded).

Additional info

No response