Repository: paritytech/substrate
File: frame/nfts/src/features/roles.rs (Rust)

Code before:
// This file is part of Substrate.

// Copyright (C) 2022 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// 	http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use crate::*;
use itertools::Itertools;

impl<T: Config<I>, I: 'static> Pallet<T, I> {
	pub fn has_role(
		collection_id: &T::CollectionId,
		account_id: &T::AccountId,
		role: CollectionRole,
	) -> bool {
		CollectionRoleOf::<T, I>::get(&collection_id, &account_id)
			.map_or(false, |roles| roles.values().contains(role))
	}

	pub fn group_roles_by_account(
		input: Vec<(T::AccountId, CollectionRole)>,
	) -> Vec<(T::AccountId, CollectionRoles)> {
		let mut sorted_by_account = input.clone();
		sorted_by_account.sort_by(|a, b| a.0.cmp(&b.0));

		let mut result = vec![];
		for (account, group) in &sorted_by_account.into_iter().group_by(|elt| elt.0.clone()) {
			let mut roles = CollectionRoles::empty().values();
			for (_, role) in group.collect::<Vec<(T::AccountId, CollectionRole)>>() {
				roles.insert(role);
			}
			result.push((account, CollectionRoles(roles)));
		}
		result
	}
}

Review comment (suggestion):
With multiple files the pallet structure looks unfamiliar to me.
The way we structure the code is a matter of taste, but keeping the structure similar within one project helps a lot.

I would rather keep the structure similar to the other pallets or have a well defined proposal with the new structure.

When I see this new structure, I have many questions, like why i see see impl of one type Pallet in different modules, why we dont have roles types in roles module, whats difference between functions.rs and features/*?

_This is not a blocker._

Code after:
// This file is part of Substrate.

// Copyright (C) 2022 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// 	http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use crate::*;
use frame_support::pallet_prelude::*;
use sp_std::collections::btree_map::BTreeMap;

impl<T: Config<I>, I: 'static> Pallet<T, I> {
	/// Clears all the roles in a specified collection.
	///
	/// - `collection_id`: A collection to clear the roles in.
	///
	/// Throws an error if some of the roles were left in storage.
	/// This means the `CollectionRoles::max_roles()` needs to be adjusted.
	pub(crate) fn clear_roles(collection_id: &T::CollectionId) -> Result<(), DispatchError> {
		let res = CollectionRoleOf::<T, I>::clear_prefix(
			&collection_id,
			CollectionRoles::max_roles() as u32,
			None,
		);
		ensure!(res.maybe_cursor.is_none(), Error::<T, I>::RolesNotCleared);
		Ok(())
	}

	/// Returns true if a specified account has a provided role within that collection.
	///
	/// - `collection_id`: A collection to check the role in.
	/// - `account_id`: An account to check the role for.
	/// - `role`: A role to validate.
	///
	/// Returns boolean.
	pub(crate) fn has_role(
		collection_id: &T::CollectionId,