feat: [AXIMST-46] Course unit - Drag And Drop For Xblocks

[PKulkoRaccoonGang]

YouTrack
https://youtrack.raccoongang.com/issue/AXIMST-46

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (ts-develop@d835b8f). Click here to learn what that means.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@              Coverage Diff              @@
##             ts-develop     #158   +/-   ##
=============================================
  Coverage              ?   89.58%           
=============================================
  Files                 ?      663           
  Lines                 ?    11014           
  Branches              ?     2307           
=============================================
  Hits                  ?     9867           
  Misses                ?     1112           
  Partials              ?       35           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[monteri]

The ErrorAlert component is imported twice. Remove one of the imports to clean up the code.

[PKulkoRaccoonGang]

I see only one import)

[monteri]

Right, my bad

[monteri]

Suggested change

	handleXBlockDragAndDrop,
	} = useCourseUnit({ courseId, blockId });
	const initialXBlocksData = useMemo(() => courseVerticalChildren.children ?? [], [courseVerticalChildren.children]);
	const [unitXBlocks, setUnitXBlocks] = useState(initialXBlocksData);
	useEffect(() => {
	document.title = getPageHeadTitle('', unitTitle);
	}, [unitTitle]);
	useEffect(() => {
	setUnitXBlocks(courseVerticalChildren.children);
	}, [courseVerticalChildren.children]);
	const {
	isShow: isShowProcessingNotification,
	title: processingNotificationTitle,
	} = useSelector(getProcessingNotification);
	if (isLoading) {
	return <Loading />;
	}
	if (sequenceStatus === RequestStatus.FAILED) {
	return (
	<Container size="xl" className="course-unit px-4 mt-4">
	<ConnectionErrorAlert />
	</Container>
	);
	}
	const finalizeXBlockOrder = (newXBlocks) => {
	handleXBlockDragAndDrop(newXBlocks.map(xBlock => xBlock.id), () => {
	setUnitXBlocks(initialXBlocksData);
	});
	};

[monteri]

try this, should work

[PKulkoRaccoonGang]

I left a self-invoking function (it is required for DraggableList), the rest of your suggestions are fully taken into account

[monteri]

Suggested change

	it('check xblock list is restored to original order when API call fails', async () => {
	it('checks xblock list is restored to original order when API call fails', async () => {

[PKulkoRaccoonGang]

Corrected

[monteri]

Suggested change

	/**
	* Updates the 'id' property of objects in the data structure using the 'blockId' value where present.
	* @param {Object} data - The original data structure to be updated.
	* @returns {Object} - The updated data structure with updated 'id' values.
	*/
	export const updateXBlockBlockIdToId = (data) => {
	if (!data?.children?.length) {
	return data;
	}
	const updatedData = { ...data };
	updatedData.children.forEach((item, index) => {
	if (item.blockId) {
	updatedData.children[index] = { ...item, id: item.blockId };
	}
	const { userPartitionInfo } = item;
	if (userPartitionInfo?.selectablePartitions) {
	updatedData.children[index].userPartitionInfo = {
	...userPartitionInfo,
	selectablePartitions: userPartitionInfo.selectablePartitions.map(partition => ({
	...partition,
	groups: partition.groups.map(group => (group.id ? { ...group, id: group.blockId } : group)),
	})),
	};
	}
	});
	return updatedData;
	};
	export const updateXBlockBlockIdToId = (data) => {
	// Base case: if data is not an object or is null, return it unmodified
	if (typeof data !== 'object' || data === null) {
	return data;
	}
	// Process arrays recursively
	if (Array.isArray(data)) {
	return data.map(updateXBlockBlockIdToId);
	}
	// For objects, create a new object to accumulate the results
	const updatedData = {};
	Object.keys(data).forEach(key => {
	const value = data[key];
	// If the current property is 'children' or 'selectablePartitions' or 'groups', process it recursively
	if (key === 'children' || key === 'selectablePartitions' || key === 'groups') {
	updatedData[key] = updateXBlockBlockIdToId(value);
	} else if (key === 'blockId') {
	// Replace 'blockId' with 'id', maintaining other properties
	updatedData['id'] = value;
	} else {
	// Copy other properties unchanged
	updatedData[key] = value;
	}
	});
	// Special handling for objects with both 'id' and 'blockId' to ensure 'blockId' takes precedence
	if ('blockId' in data) {
	updatedData['id'] = data['blockId'];
	}
	return updatedData;
	};

Check if this is more effecient implementation

[PKulkoRaccoonGang]

Added and deleted unnecessary comments
Thanks

[monteri]

Suggested change

	const handleXBlockDragAndDrop = (xBlockListIds, restoreCallback) => {
	dispatch(setXBlockOrderListQuery(blockId, xBlockListIds, restoreCallback));
	const handleXBlockDragAndDrop = (xblockListIds, restoreCallback) => {
	dispatch(setXBlockOrderListQuery(blockId, xblockListIds, restoreCallback));

[GlugovGrGlib]

Carmel Case is tricky, but I agree with Vlad we should do either the whole word in lowercase or both letters XB in uppercase

refs:
https://github.com/openedx/frontend-app-learning/blob/9dd79ca77ecb46dd0ea616945daa21d49fea5ee8/src/courseware/course/sequence/Unit/urls.js#L16
https://github.com/openedx/frontend-lib-content-components/blob/f33a3b5521d709ae4b0a98a5bdd9744060177816/addXblock.js#L5
https://github.com/openedx/edx-platform/blob/fa66728a67705723bd9cc74d3ccc9ca3bda4b7ae/cms/static/js/views/utils/xblock_utils.js#L184

[PKulkoRaccoonGang]

corrected, thanks guys

[monteri]

Suggested change

	export function setXBlockOrderListQuery(blockId, xBlockListIds, restoreCallback) {
	export function setXBlockOrderListQuery(blockId, xblockListIds, restoreCallback) {

[PKulkoRaccoonGang]

Done

[monteri]

Suggested change

	reorderXBlockList: (state, { payload }) => {
	const xBlockList = [...state.courseVerticalChildren.children];
	xBlockList.sort((a, b) => payload.indexOf(a.id) - payload.indexOf(b.id));
	state.courseVerticalChildren.children = [...xBlockList];
	},
	reorderXBlockList: (state, { payload }) => {
	// Create a map for payload IDs to their index for O(1) lookups
	const indexMap = new Map(payload.map((id, index) => [id, index]));
	// Directly sort the children based on the order defined in payload
	// This avoids the need to copy the array beforehand
	state.courseVerticalChildren.children.sort((a, b) => {
	return (indexMap.get(a.id) || 0) - (indexMap.get(b.id) || 0);
	});
	},

indexOf inside sort function will be ineffective.

[PKulkoRaccoonGang]

I moved this code from another page, but your suggestion looks great!

[GlugovGrGlib]

Was this just for debug, or where this is used?

[PKulkoRaccoonGang]

Good catch! Thanks

[GlugovGrGlib]

Looks ok, except comments above