Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Multiple SubCollections of the Same Type #32

Closed
elersong opened this issue Jul 13, 2024 · 0 comments · Fixed by #45
Closed

Support Multiple SubCollections of the Same Type #32

elersong opened this issue Jul 13, 2024 · 0 comments · Fixed by #45
Assignees
Labels
bug Something isn't working from_willyovale An issue described in original project, but never implemented

Comments

@elersong
Copy link
Owner

Description

Defining multiple subcollections of the same type within a Fireorm model causes an error. While there are workarounds, such as extending the class, this should ideally be supported without errors. The issue seems to stem from the way MetadataStorage.setCollection() evaluates children before parents and updates segments.

Steps to Reproduce

  1. Create a model with multiple subcollections of the same type.
  2. Attempt to use the model in Fireorm.
  3. Observe the error caused by having multiple subcollections of the same type.

Expected Behavior

Fireorm should support multiple subcollections of the same type within a model without causing errors.

Actual Behavior

An error is thrown when defining multiple subcollections of the same type within a model.

Acceptance Criteria

  • Modify MetadataStorage.setCollection() to correctly handle multiple subcollections of the same type.
  • Ensure that parent and child collections are evaluated and updated correctly.
  • Add unit tests to validate the support for multiple subcollections of the same type.

Additional Context

  • April 26, 2021: Initial issue raised about the error caused by multiple subcollections of the same type.
  • April 6, 2022: Additional comments about the need for a solution and suggestions for workarounds.
  • April 7, 2022: Reference to a related issue.

Proposed API Changes

  1. Update MetadataStorage.setCollection:

    • Modify the MetadataStorage.setCollection() method to correctly handle multiple subcollections of the same type.
    class MetadataStorage {
      setCollection(collection: any) {
        // Logic to handle multiple subcollections of the same type
        // Ensure parent and child collections are evaluated and updated correctly
      }
    }
  2. Support Multiple SubCollections:

    • Ensure the model can support multiple subcollections of the same type without errors.
    @Collection()
    class Band {
      id: string;
      name: string;
      @SubCollection(Album, 'albums')
      albums: ISubCollection<Album>;
      @SubCollection(Album, 'singles')
      singles: ISubCollection<Album>;
    }
  3. Unit Tests:

    • Add unit tests to validate the correct handling of multiple subcollections of the same type.
    test('should support multiple subcollections of the same type', () => {
      const bandRepository = getRepository(Band);
      const band = new Band();
      band.id = 'band1';
      band.name = 'Test Band';
    
      const album = new Album();
      album.id = 'album1';
      album.name = 'Test Album';
    
      const single = new Album();
      single.id = 'single1';
      single.name = 'Test Single';
    
      band.albums.create(album);
      band.singles.create(single);
    
      expect(band.albums.findById('album1').name).toBe('Test Album');
      expect(band.singles.findById('single1').name).toBe('Test Single');
    });

Example Implementation

class Album {
  id: string;
  name: string;
}

@Collection()
class Band {
  id: string;
  name: string;
  @SubCollection(Album, 'albums')
  albums: ISubCollection<Album>;
  @SubCollection(Album, 'singles')
  singles: ISubCollection<Album>;
}

const bandRepository = getRepository(Band);

const band = new Band();
band.id = 'band1';
band.name = 'Test Band';

const album = new Album();
album.id = 'album1';
album.name = 'Test Album';

const single = new Album();
single.id = 'single1';
single.name = 'Test Single';

bandRepository.create(band);
band.albums.create(album);
band.singles.create(single);

const fetchedAlbum = band.albums.findById('album1');
const fetchedSingle = band.singles.findById('single1');

console.log(fetchedAlbum.name); // Output: 'Test Album'
console.log(fetchedSingle.name); // Output: 'Test Single'

Original Issue

@elersong elersong added bug Something isn't working from_willyovale An issue described in original project, but never implemented labels Jul 13, 2024
@elersong elersong self-assigned this Jul 15, 2024
@elersong elersong linked a pull request Jul 21, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working from_willyovale An issue described in original project, but never implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant