Avoid including domain logic in the repository layer

Question

In my current application I need to create an api GET route "/inactive-users" which should return users that are archived ("isArchived" ) and are inactive at the same time More than one month old ("lastVisitedDate" field should be older than new Date() - 1 month).

A layered architecture (controller/service/repository) is used and the domain model is poor. What should I do in this case?

I see there are two possible approaches.

1 - Create a generic repository method to get the user and pass it the user fields we need.

@Injectable()
export class UserRepository {
  constructor(private readonly prisma: PrismaService) { }

  findAll = async ( where: { user: Partial<User>; dateTreshold: Date } ): Promise<User[]> => {
    const users = await this.prisma.user.findMany(
      { where: {...user}, lastVisitedDate: { lt: dateTreshold }; // lt - less than
    );
        
    return users.map(user => new User(user));
  }
}

@Injectable()
export class UserService {
  constructor(private readonly userRepository: UserRepository) {}
  
  getInactiveUsers = async () => {
    return this.userRepository.findAll(
      { where: {user: {isArchived: true}, dateTreshold: "// calculatedDate //"}
    ) 
  }
}

2 - Create the repository method exactly to retrieve inactive users, the method will know which fields it should request.

@Injectable()
export class UserRepository {
  constructor(private readonly prisma: PrismaService) { }

  getInactiveUsers = async (): Promise<User[]> => {
    const users = await this.prisma.user.findMany(
      { where: {isArchived: false, lastVisitedDate: { lt: "// calculatedDate //" }}; // lt - less than
    );
        
    return users.map(user => new User(user));
  }
}

@Injectable()
export class UserService {
  constructor(private readonly userRepository: UserRepository) {}
  
  getInactiveUsers = async () => {
    return this.userRepository.getInactiveUsers() 
  }
}

Which way is better? The first one looks good to me because in this case the repository knows nothing about the "inactive" user's domain understanding. But at the same time - building such a reactive approach can be quite difficult.

The second way is easier to build, but at the same time - it has some "business" logic understanding and knows that inactive users are those for which "isArchived" equals false. Also, this repository method knows how many days we need to use.

Which option should be chosen in this case? Or maybe there are other ways to build this thing?

Answer 1

The correct way to isolate this problem is that the repository only knows about the data elements in the repository. This doesn't mean you can't have multiple entry points into your repository, there may be many different queries all accessing the same "table".

This doesn't mean you need a full QBE like the first approach, keep it simple. There is a query that encapsulates the database level but still requires what it needs.

In this case you should have a signature passing isArchived and lastVisitedDate parameters. Something like QueryUsersByStatusAndLastVisited. This way the repository handles all the bits involved in retrieving the data, but has no logic as to why they are being retrieved. It will be "dumb", with all the smart bits encapsulated in the service layer.