From e9ab11c8bb0f5dba45002030ca64494d9ec54750 Mon Sep 17 00:00:00 2001 From: Branislav Nohaj Date: Sat, 4 Oct 2025 10:58:36 +0200 Subject: [PATCH] added better comments --- .../add_exercise/image_details_form.dart | 67 +++++++++++++++---- .../add_exercise/steps/step5images.dart | 48 ++++++++++--- .../add_exercise_image_withDetail_test.dart | 47 +++++++++++-- 3 files changed, 135 insertions(+), 27 deletions(-) diff --git a/lib/widgets/add_exercise/image_details_form.dart b/lib/widgets/add_exercise/image_details_form.dart index 315d270e..805fb586 100644 --- a/lib/widgets/add_exercise/image_details_form.dart +++ b/lib/widgets/add_exercise/image_details_form.dart @@ -1,10 +1,24 @@ import 'dart:io'; import 'package:flutter/material.dart'; -/// Form for collecting image metadata including license information +/// Form for collecting CC BY-SA 4.0 license metadata for exercise images /// -/// Displayed after image selection in Step 5 of exercise creation -/// Collects: title, source URL, author info, derivative work info, and image style +/// This form is displayed after image selection in Step 5 of exercise creation. +/// It collects all required and optional license attribution fields: +/// +/// Required by CC BY-SA 4.0: +/// - Author name +/// - License type (implicitly CC BY-SA 4.0) +/// +/// Optional but recommended: +/// - Title (helps identify the image) +/// - Source URL (where image was found) +/// - Author URL (author's website/profile) +/// - Derivative source URL (if modified from another work) +/// - Image style (PHOTO, 3D, LINE, LOW-POLY, OTHER) +/// +/// All metadata is sent to the API's /exerciseimage endpoint along with +/// the image file when the exercise is submitted. class ImageDetailsForm extends StatefulWidget { final File imageFile; final Function(File image, Map details) onAdd; @@ -24,12 +38,15 @@ class ImageDetailsForm extends StatefulWidget { class _ImageDetailsFormState extends State { final _formKey = GlobalKey(); + // Text controllers for license metadata fields final _titleController = TextEditingController(); - final _sourceLinkController = TextEditingController(); - final _authorController = TextEditingController(); - final _authorLinkController = TextEditingController(); - final _originalSourceController = TextEditingController(); + final _sourceLinkController = TextEditingController(); // license_object_url in API + final _authorController = TextEditingController(); // license_author in API + final _authorLinkController = TextEditingController(); // license_author_url in API + final _originalSourceController = TextEditingController(); // license_derivative_source_url in API + /// Currently selected image type + /// Maps to API 'style' field: PHOTO=1, 3D=2, LINE=3, LOW-POLY=4, OTHER=5 String _selectedImageType = 'PHOTO'; final List> _imageTypes = [ @@ -50,8 +67,14 @@ class _ImageDetailsFormState extends State { super.dispose(); } - /// Maps UI image type selection to API style value - /// PHOTO=1, 3D=2, LINE=3, LOW-POLY=4, OTHER=5 + /// Maps UI image type selection to API 'style' field value + /// + /// API expects numeric string: + /// - PHOTO = '1' + /// - 3D = '2' + /// - LINE = '3' + /// - LOW-POLY = '4' + /// - OTHER = '5' String _getStyleValue() { switch (_selectedImageType) { case 'PHOTO': @@ -65,7 +88,7 @@ class _ImageDetailsFormState extends State { case 'OTHER': return '5'; default: - return '1'; + return '1'; // Default to PHOTO if unknown } } @@ -90,6 +113,7 @@ class _ImageDetailsFormState extends State { _buildImagePreview(), const SizedBox(height: 24), + // License title field - helps identify the image _buildTextField( controller: _titleController, label: 'Title', @@ -97,6 +121,7 @@ class _ImageDetailsFormState extends State { ), const SizedBox(height: 16), + // Source URL - where the image was found (license_object_url in API) _buildTextField( controller: _sourceLinkController, label: 'Link to the source website, if available', @@ -105,6 +130,7 @@ class _ImageDetailsFormState extends State { ), const SizedBox(height: 16), + // Author name - required for proper CC BY-SA attribution _buildTextField( controller: _authorController, label: 'Author(s)', @@ -112,6 +138,7 @@ class _ImageDetailsFormState extends State { ), const SizedBox(height: 16), + // Author's website/profile URL _buildTextField( controller: _authorLinkController, label: 'Link to author website or profile, if available', @@ -120,6 +147,7 @@ class _ImageDetailsFormState extends State { ), const SizedBox(height: 16), + // Original source if this is a derivative work (modified from another image) _buildTextField( controller: _originalSourceController, label: 'Link to the original source, if this is a derivative work', @@ -202,7 +230,10 @@ class _ImageDetailsFormState extends State { ); } - /// Info box explaining what constitutes a derivative work + /// Informational box explaining what constitutes a derivative work + /// + /// Important for CC BY-SA compliance - users need to understand when + /// they must cite the original work they modified Widget _buildDerivativeWorkNote() { return Container( padding: const EdgeInsets.all(12), @@ -234,7 +265,10 @@ class _ImageDetailsFormState extends State { ); } - /// Selector for image style (PHOTO, 3D, LINE, LOW-POLY, OTHER) + /// Visual selector for image style/type + /// + /// Allows user to categorize the image as PHOTO, 3D render, LINE drawing, + /// LOW-POLY art, or OTHER. This helps users find appropriate exercise images. Widget _buildImageTypeSelector() { return Column( crossAxisAlignment: CrossAxisAlignment.start, @@ -298,7 +332,10 @@ class _ImageDetailsFormState extends State { ); } - /// Warning box about CC BY-SA 4.0 license requirements + /// Legal notice about CC BY-SA 4.0 license + /// + /// Critical for compliance - informs user that by uploading, they're + /// releasing the image under CC BY-SA 4.0 and must have rights to do so Widget _buildLicenseInfo() { return Container( padding: const EdgeInsets.all(12), @@ -359,11 +396,12 @@ class _ImageDetailsFormState extends State { onPressed: () { if (_formKey.currentState!.validate()) { // Build details map with API field names + // Style is always included, other fields only if non-empty final details = { 'style': _getStyleValue(), }; - // Add only non-empty fields + // Add optional fields only if user provided values final title = _titleController.text.trim(); if (title.isNotEmpty) { details['license_title'] = title; @@ -389,6 +427,7 @@ class _ImageDetailsFormState extends State { details['license_derivative_source_url'] = derivativeUrl; } + // Pass image and metadata back to parent widget.onAdd(widget.imageFile, details); } }, diff --git a/lib/widgets/add_exercise/steps/step5images.dart b/lib/widgets/add_exercise/steps/step5images.dart index 413be166..8ccaf83a 100644 --- a/lib/widgets/add_exercise/steps/step5images.dart +++ b/lib/widgets/add_exercise/steps/step5images.dart @@ -8,6 +8,17 @@ import 'package:wger/widgets/add_exercise/mixins/image_picker_mixin.dart'; import 'package:wger/widgets/add_exercise/preview_images.dart'; import 'package:wger/widgets/add_exercise/image_details_form.dart'; +/// Step 5 of exercise creation wizard - Image upload with license metadata +/// +/// This step allows users to add exercise images with proper CC BY-SA 4.0 license +/// attribution. Unlike the previous implementation that uploaded images directly, +/// this version collects license metadata (title, author, URLs) before adding images. +/// +/// Flow: +/// 1. User picks image from camera/gallery +/// 2. ImageDetailsForm is shown to collect license metadata +/// 3. Image + metadata is stored in AddExerciseProvider +/// 4. Final upload happens in Step 6 when user clicks "Submit" class Step5Images extends StatefulWidget { final GlobalKey formkey; const Step5Images({required this.formkey}); @@ -17,9 +28,16 @@ class Step5Images extends StatefulWidget { } class _Step5ImagesState extends State with ExerciseImagePickerMixin { + /// Currently selected image waiting for metadata input + /// When non-null, ImageDetailsForm is displayed instead of image picker File? _currentImageToAdd; - /// Pick image and show details form for license metadata + /// Pick image from camera or gallery and show metadata collection form + /// + /// Validates file format (jpg, jpeg, png, webp) and size (<20MB) before + /// showing the form. Invalid files are rejected with a snackbar message. + /// + /// [pickFromCamera] - If true, opens camera; otherwise opens gallery void _pickAndShowImageDetails(BuildContext context, {bool pickFromCamera = false}) async { final imagePicker = ImagePicker(); @@ -33,7 +51,7 @@ class _Step5ImagesState extends State with ExerciseImagePickerMixin if (selectedImage != null) { final imageFile = File(selectedImage.path); - // Validate file type and size + // Validate file type - only common image formats accepted bool isFileValid = true; String errorMessage = ''; @@ -44,6 +62,7 @@ class _Step5ImagesState extends State with ExerciseImagePickerMixin errorMessage = "Select only 'jpg', 'jpeg', 'png', 'webp' files"; } + // Validate file size - 20MB limit matches server-side restriction final fileSizeInMB = imageFile.lengthSync() / 1024 / 1024; if (fileSizeInMB > 20) { isFileValid = false; @@ -59,17 +78,25 @@ class _Step5ImagesState extends State with ExerciseImagePickerMixin return; } - // Show details form for valid image + // Show metadata collection form for valid image setState(() { _currentImageToAdd = imageFile; }); } } - /// Add image with license metadata to provider + /// Add image with its license metadata to the provider + /// + /// Called when user clicks "ADD" in ImageDetailsForm. The image and metadata + /// are stored locally in AddExerciseProvider and will be uploaded together + /// when the exercise is submitted in Step 6. + /// + /// [image] - The image file to add + /// [details] - Map containing license fields (license_title, license_author, etc.) void _addImageWithDetails(File image, Map details) { final provider = context.read(); + // Store image with metadata - actual upload happens in addExercise() provider.addExerciseImages( [image], title: details['license_title'], @@ -80,6 +107,7 @@ class _Step5ImagesState extends State with ExerciseImagePickerMixin style: details['style'] ?? '1', ); + // Reset form state setState(() { _currentImageToAdd = null; }); @@ -93,6 +121,7 @@ class _Step5ImagesState extends State with ExerciseImagePickerMixin ); } + /// Cancel metadata input and return to image picker void _cancelImageAdd() { setState(() { _currentImageToAdd = null; @@ -105,7 +134,7 @@ class _Step5ImagesState extends State with ExerciseImagePickerMixin key: widget.formkey, child: Column( children: [ - // Show license notice when not adding image + // License notice - shown when not entering metadata if (_currentImageToAdd == null) Padding( padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0), @@ -116,7 +145,7 @@ class _Step5ImagesState extends State with ExerciseImagePickerMixin ), ), - // Show image details form when image is selected + // Metadata collection form - shown when image is selected if (_currentImageToAdd != null) ImageDetailsForm( imageFile: _currentImageToAdd!, @@ -124,12 +153,12 @@ class _Step5ImagesState extends State with ExerciseImagePickerMixin onCancel: _cancelImageAdd, ), - // Show image picker or preview when no image is being added + // Image picker or preview - shown when not entering metadata if (_currentImageToAdd == null) Consumer( builder: (ctx, provider, __) { if (provider.exerciseImages.isNotEmpty) { - // Show preview of existing images + // Show preview of images that have been added with metadata return Column( children: [ PreviewExerciseImages( @@ -145,7 +174,7 @@ class _Step5ImagesState extends State with ExerciseImagePickerMixin ); } - // Show empty state with camera/gallery buttons + // Empty state - no images added yet return Column( children: [ const SizedBox(height: 20), @@ -162,6 +191,7 @@ class _Step5ImagesState extends State with ExerciseImagePickerMixin ), ), const SizedBox(height: 24), + // Camera and Gallery buttons Row( mainAxisAlignment: MainAxisAlignment.center, children: [ diff --git a/test/exercises/add_exercise_image_withDetail_test.dart b/test/exercises/add_exercise_image_withDetail_test.dart index 5e4435d3..b7428085 100644 --- a/test/exercises/add_exercise_image_withDetail_test.dart +++ b/test/exercises/add_exercise_image_withDetail_test.dart @@ -4,6 +4,16 @@ import 'package:wger/providers/add_exercise.dart'; import '../core/settings_test.mocks.dart'; +/// Unit tests for AddExerciseProvider image metadata handling +/// +/// Tests the functionality added for issue #931 - storing and managing +/// CC BY-SA 4.0 license metadata alongside exercise images. +/// +/// Key areas tested: +/// - Adding images with complete/partial/no metadata +/// - Edge cases (empty lists, duplicates, special characters) +/// - State management (clear, remove) +/// - Image ordering and batch operations void main() { late MockWgerBaseProvider mockBaseProvider; late AddExerciseProvider provider; @@ -14,6 +24,8 @@ void main() { }); group('Image metadata handling', () { + /// Verify that all CC BY-SA license fields are stored correctly + /// Tests: title, author, authorUrl, sourceUrl, derivativeSourceUrl, style test('should store image with all license fields', () { final mockFile = File('test.jpg'); @@ -24,27 +36,31 @@ void main() { authorUrl: 'https://test.com/author', sourceUrl: 'https://source.com', derivativeSourceUrl: 'https://derivative.com', - style: '4', + style: '4', // LOW-POLY ); expect(provider.exerciseImages.length, 1); expect(provider.exerciseImages.first.path, mockFile.path); }); + /// License fields are optional - provider should handle null/empty values + /// Only non-empty fields should be included in the metadata map test('should handle empty fields gracefully', () { final mockFile = File('test2.jpg'); provider.addExerciseImages( [mockFile], title: 'Only Title', - author: null, - authorUrl: '', - style: '2', + author: null, // null value + authorUrl: '', // empty string + style: '2', // 3D ); expect(provider.exerciseImages.length, 1); }); + /// Each image can have different metadata - test storing multiple + /// images with unique license information test('should handle multiple images with different metadata', () { final file1 = File('image1.jpg'); final file2 = File('image2.jpg'); @@ -57,6 +73,8 @@ void main() { expect(provider.exerciseImages[1].path, file2.path); }); + /// Test all 5 image style types defined by the API + /// 1=PHOTO, 2=3D, 3=LINE, 4=LOW-POLY, 5=OTHER test('should handle all image style types', () { final styles = ['1', '2', '3', '4', '5']; @@ -70,6 +88,7 @@ void main() { expect(provider.exerciseImages.length, 5); }); + /// If no style is specified, should default to '1' (PHOTO) test('should use default style when not specified', () { final mockFile = File('default.jpg'); @@ -78,12 +97,15 @@ void main() { expect(provider.exerciseImages.length, 1); }); + /// Edge case: calling addExerciseImages with empty list should not crash test('should handle empty image list', () { provider.addExerciseImages([]); expect(provider.exerciseImages.length, 0); }); + /// Allows adding the same file multiple times with different metadata + /// (e.g., different crops or edits of the same original image) test('should handle adding same file multiple times', () { final mockFile = File('same.jpg'); @@ -93,6 +115,8 @@ void main() { expect(provider.exerciseImages.length, 2); }); + /// Removing an image should also remove its associated metadata + /// to prevent memory leaks test('should remove image and its metadata', () { final mockFile = File('to_remove.jpg'); provider.addExerciseImages([mockFile], title: 'Will be removed', style: '1'); @@ -104,6 +128,8 @@ void main() { expect(provider.exerciseImages.length, 0); }); + /// Attempting to remove a non-existent image should throw StateError + /// (from firstWhere with no orElse) test('should handle removing non-existent image gracefully', () { expect( () => provider.removeExercise('nonexistent.jpg'), @@ -111,6 +137,7 @@ void main() { ); }); + /// clear() should reset all state including images and metadata test('should clear all images and metadata', () { provider.addExerciseImages([File('image1.jpg')], title: 'Image 1'); provider.addExerciseImages([File('image2.jpg')], title: 'Image 2'); @@ -122,6 +149,7 @@ void main() { expect(provider.exerciseImages.length, 0); }); + /// Clearing an already empty list should not cause errors test('should handle clearing empty list', () { expect(provider.exerciseImages.length, 0); @@ -130,6 +158,8 @@ void main() { expect(provider.exerciseImages.length, 0); }); + /// Images should be stored in the order they were added + /// Important for display consistency test('should preserve image order', () { final file1 = File('first.jpg'); final file2 = File('second.jpg'); @@ -144,6 +174,8 @@ void main() { expect(provider.exerciseImages[2].path, 'third.jpg'); }); + /// Multiple images can be added in a single call with shared metadata + /// Useful for bulk uploads from the same source test('should handle batch adding multiple images at once', () { final files = [ File('batch1.jpg'), @@ -156,6 +188,7 @@ void main() { expect(provider.exerciseImages.length, 3); }); + /// Removing one image from a set should not affect others test('should allow removing specific image from multiple images', () { final file1 = File('keep1.jpg'); final file2 = File('remove.jpg'); @@ -174,6 +207,8 @@ void main() { expect(provider.exerciseImages[1].path, file3.path); }); + /// Test with extremely long strings (1000 chars) to ensure no + /// buffer overflow or validation issues test('should handle very long metadata strings', () { final mockFile = File('long.jpg'); final longString = 'a' * 1000; @@ -188,6 +223,8 @@ void main() { expect(provider.exerciseImages.length, 1); }); + /// Unicode characters, emojis, and URL-encoded strings should all work + /// Tests international character support test('should handle special characters in metadata', () { final mockFile = File('special.jpg'); @@ -203,6 +240,8 @@ void main() { }); group('State management', () { + /// clear() should reset ALL provider state, not just images + /// Ensures no data leaks between exercises test('should reset all state after clear', () { provider.exerciseNameEn = 'Test Exercise'; provider.descriptionEn = 'Description';