♻️ Add a modifier to ensure room exists before any operation

README.md

@@ -38,5 +38,3 @@ This DApp is composed of 2 main folders:
 4. The webapp is available at http://localhost:8080/
 
 ## To Do
-
-* Add a modifier to ensure room exists before any operation

blockchain/contracts/BookARoom.sol

@@ -29,30 +29,29 @@ contract BookARoom {
         emit RoomAdded(_roomName, getRoomsNumber() - 1);
     }
 
-    function deleteRoom(uint indexToDelete) public isOwner {
-        require(rooms.length > indexToDelete, "Out of bounds");
-        for (uint i = indexToDelete; i < rooms.length - 1; i++) {
+    function deleteRoom(uint _indexToDelete) public isOwner roomExists(_indexToDelete) {
+        for (uint i = _indexToDelete; i < rooms.length - 1; i++) {
             rooms[i] = rooms[i + 1];
         }
         rooms.pop();
         emit RoomDeleted();
     }
 
-    function nameRoom(uint _roomId, string memory _roomName) public isOwner {
+    function nameRoom(uint _roomId, string memory _roomName) public isOwner roomExists(_roomId) {
         rooms[_roomId].name = _roomName;
         emit RoomRenamed(_roomName);
     }
 
-    function getPlanning(uint _roomId) public view returns (address[24] memory){
+    function getPlanning(uint _roomId) public view roomExists(_roomId) returns (address[24] memory){
         return rooms[_roomId].planning;
     }
 
-    function bookARoom(uint _roomId, uint _hour) public roomIsFree(_roomId, _hour) {
+    function bookARoom(uint _roomId, uint _hour) public roomExists(_roomId) roomIsFree(_roomId, _hour) {
         rooms[_roomId].planning[_hour] = msg.sender;
         emit BookingConfirmed(msg.sender, rooms[_roomId].name, _hour);
     }
 
-    function cancelBooking(uint _roomId, uint _hour) public isBooker(_roomId, _hour) {
+    function cancelBooking(uint _roomId, uint _hour) public roomExists(_roomId) isBooker(_roomId, _hour) {
         rooms[_roomId].planning[_hour] = address(0);
         emit BookingCancelled(msg.sender, rooms[_roomId].name, _hour);
     }
@@ -76,4 +75,9 @@ contract BookARoom {
         _;
     }
 
+    modifier roomExists(uint _index) {
+        require(rooms.length > _index, "This room doesn't exist");
+        _;
+    }
+
 }

blockchain/test/BookARoom.js

@@ -53,6 +53,9 @@ contract('BookARoom', (accounts) => {
         await truffleAssert.reverts(bookARoomContract.deleteRoom(0, {from: secondAccount}), 'This function is restricted to the contract\'s owner');
     });
 
+    it('shouldn\'t be able to delete a room if it doesn\'t exist', async () => {
+        await truffleAssert.reverts(bookARoomContract.deleteRoom(2), 'This room doesn\'t exist');
+    });
 
     it('should name a room and emit an event', async () => {
         const tx = await bookARoomContract.nameRoom(ROOM_ID, NEW_ROOM_NAME);
@@ -68,6 +71,22 @@ contract('BookARoom', (accounts) => {
         await truffleAssert.reverts(bookARoomContract.nameRoom(ROOM_ID, NEW_ROOM_NAME, {from: secondAccount}), 'This function is restricted to the contract\'s owner');
     });
 
+    it('shouldn\'t be able to rename a room if it doesn\'t exist', async () => {
+        await truffleAssert.reverts(bookARoomContract.nameRoom(2, NEW_ROOM_NAME), 'This room doesn\'t exist');
+    });
+
+    it('should be able to get a room\'s planning', async () => {
+        const expectedPlanning=Array(24).fill('0x0000000000000000000000000000000000000000');
+        const planningRoom = await bookARoomContract.getPlanning(ROOM_ID);
+
+        assert.isNotNull(planningRoom);
+        assert.deepEqual(planningRoom, expectedPlanning, 'A new room should have an empty planning');
+    });
+
+    it('shouldn\'t be able to get a room\'s planning if it doesn\'t exist', async () => {
+        await truffleAssert.reverts(bookARoomContract.getPlanning(2), 'This room doesn\'t exist');
+    });
+
     it('should book a room and emit an event', async () => {
         const tx = await bookARoomContract.bookARoom(ROOM_ID, HOUR, {from: firstAccount});
         const planningRoom0 = await bookARoomContract.getPlanning(ROOM_ID);
@@ -86,6 +105,10 @@ contract('BookARoom', (accounts) => {
         await truffleAssert.reverts(bookARoomContract.bookARoom(ROOM_ID, HOUR, {from: secondAccount}), 'This hour is already booked');
     });
 
+    it('shouldn\'t be able to book a room if it doesn\'t exist', async () => {
+        await truffleAssert.reverts(bookARoomContract.bookARoom(2, HOUR, {from: secondAccount}), 'This room doesn\'t exist');
+    });
+
     it('should cancel a booking and emit an event', async () => {
         await bookARoomContract.bookARoom(ROOM_ID, HOUR, {from: firstAccount});
         const tx = await bookARoomContract.cancelBooking(ROOM_ID, HOUR, {from: firstAccount});
@@ -104,4 +127,8 @@ contract('BookARoom', (accounts) => {
 
         await truffleAssert.reverts(bookARoomContract.cancelBooking(ROOM_ID, HOUR, {from: secondAccount}), 'This is not your booking');
     });
+
+    it('shouldn\'t be able to cancel a booking if the room doesn\'t exist', async () => {
+        await truffleAssert.reverts(bookARoomContract.cancelBooking(2, HOUR, {from: secondAccount}), 'This room doesn\'t exist');
+    });
 });