Feature/ia cc license #707
Conversation
IA refactor to MVI from Redux CC License Setup Screen Compose Migration IA Detail Screen CC License Integration
SpaceList screen add btn size increased
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Internet Archive feature from Redux pattern to MVI architecture while adding Creative Commons licensing support and performing cleanup of unused components. The changes streamline the navigation flow and improve code maintainability by migrating to a fragment-based approach within a unified navigation graph.
Key changes:
- Migrated Internet Archive from Redux pattern to MVI architecture with proper state management
- Added CC0 licensing option alongside existing Creative Commons licensing
- Removed deprecated activities and consolidated navigation flow to use fragments
- Updated library dependencies and performed code cleanup
Reviewed Changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Library version upgrades including AGP, test runner, Koin, and security dependencies |
| app/src/main/res/navigation/space_setup_navigation.xml | Updated navigation flow to use new Internet Archive fragments and license setup |
| app/src/main/res/layout/content_cc.xml | Added CC0 licensing option with new switch component |
| app/src/main/java/net/opendasharchive/openarchive/features/internetarchive/presentation/login/ | Migrated from Redux to MVI pattern with proper state management |
| app/src/main/java/net/opendasharchive/openarchive/features/settings/license/ | New unified license setup fragment supporting multiple space types |
| app/src/main/java/net/opendasharchive/openarchive/services/webdav/ | Removed deprecated activities and updated fragments to use navigation args |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| <TextView | ||
| android:layout_width="240dp" | ||
| android:layout_height="wrap_content" | ||
| android:textFontWeight="500" |
There was a problem hiding this comment.
Using android:textFontWeight requires API level 28+. Consider using android:textStyle="bold" or defining font weights in a custom font family for better compatibility.
| private lateinit var binding: FragmentWebdavSetupLicenseBinding | ||
|
|
||
| private var mSpaceId by Delegates.notNull<Long>() | ||
| private val args: SetupLicenseFragmentArgs by navArgs() |
There was a problem hiding this comment.
The args property is accessed before binding is initialized in onCreateView. Consider reordering these declarations or ensuring proper initialization order to avoid potential issues.
| private var mSpaceId by Delegates.notNull<Long>() | ||
| private val args: SetupLicenseFragmentArgs by navArgs() | ||
|
|
||
| private lateinit var binding: FragmentSetupLicenseBinding |
There was a problem hiding this comment.
The args property is accessed before binding is initialized in onCreateView. Consider reordering these declarations or ensuring proper initialization order to avoid potential issues.
| val space: Space = if (spaceId == -1L) { | ||
| Space(Space.Type.INTERNET_ARCHIVE) | ||
| } else { | ||
| Space.get(spaceId) ?: Space(Space.Type.INTERNET_ARCHIVE) |
There was a problem hiding this comment.
The space property is declared as val but contains mutable logic. Consider making this a function or private property with lazy initialization to better reflect its behavior and improve testability.
| val space: Space = if (spaceId == -1L) { | |
| Space(Space.Type.INTERNET_ARCHIVE) | |
| } else { | |
| Space.get(spaceId) ?: Space(Space.Type.INTERNET_ARCHIVE) | |
| private val space: Space by lazy { | |
| if (spaceId == -1L) { | |
| Space(Space.Type.INTERNET_ARCHIVE) | |
| } else { | |
| Space.get(spaceId) ?: Space(Space.Type.INTERNET_ARCHIVE) | |
| } |
|
|
||
| private fun loadSpaceData() { | ||
| try { | ||
| val metaData = if (space.metaData.isNotEmpty()) { |
There was a problem hiding this comment.
The metaData property access should be null-safe. If space.metaData is null, calling .isNotEmpty() will throw a NullPointerException. Use space.metaData?.isNotEmpty() == true instead.
| val metaData = if (space.metaData.isNotEmpty()) { | |
| val metaData = if (space.metaData?.isNotEmpty() == true) { |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
…ds in crashlytics
removed space id arg from IALoginScreen, made IADetailScreen arg space_id as required
Added Creative Common Licensing to Internet Archive
Refactored Internet Archive Feature from redux pattern to MVI
Code cleanup (removed unused activities) - since most of the screens are migrated to fragments linked in one navgraph or compose
library upgrades
minor text changes as per requirement