Hardhat-Enterprises / Deakin-Detonator-Toolkit

Deakin Detonator Toolkit redesign using Tauri, React and Mantine.
15 stars 23 forks source link

Hathim NSLookup tool updated using the baseline checklist #740

Closed hathimsh closed 2 months ago

hathimsh commented 2 months ago

During the latest code review, some significant modifications were implemented to improve the clarity and ensure the resilience of the code. The handleProcessData and handleProcessTermination functions were enhanced with comprehensive docstrings and inline comments, providing a thorough explanation of their purpose and internal mechanisms. This facilitates enhanced comprehension and upkeep of the code. In addition, the state variables and form management logic were thoroughly examined to verify their accuracy, assuring appropriate initialisation and updates. The examination also included an assessment of error handling to ensure the smooth and effective management of any issues. In addition, the tutorial component was included with a blank string to offer explicit guidance on utilising the NSLookup utility. The objective of these enhancements is to enhance the readability, maintainability, and reliability of the NSLookup component during execution.

hathimsh commented 2 months ago

Hi Micheal

I just saw the mail I don’t know what happened as all mails were in the spam folder i guess this one is old pull request i have made a pull request saying including the tutorial which has empty tutorial string and it has some more changes. Can you please check and i will also compare the suggestions that is given by you with that code. So can i make this as sprint 2 of the project?

On Fri, 16 Aug 2024 at 5:41 PM, michaeljpigott @.***> wrote:

@.**** requested changes on this pull request.

Hi @hathimsh https://github.com/hathimsh

This is a good first effort, but there is still a fair bit of work to do before this tool is baselined.

Please implement my recommended changes and push them back into your branch. The pull request will automatically update.

In src/components/NSLookupTool/NSLookupTool.tsx https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719465862 :

@@ -7,50 +7,54 @@ import { UserGuide } from "../UserGuide/UserGuide"; import { SaveOutputToTextFile_v2 } from "../SaveOutputToFile/SaveOutputToTextFile"; import { LoadingOverlayAndCancelButton } from "../OverlayAndCancelButton/OverlayAndCancelButton";

You do not need to import UserGuide because it is deprecated. You need to import checkAllCommandsAvailability, InstallationModal and RenderComponent. ⬇️ Suggested change

- +import { checkAllCommandsAvailability } from "../../utils/CommandAvailability"; +import InstallationModal from "../InstallationModal/InstallationModal"; +import { RenderComponent } from "../UserGuide/UserGuide";


In src/components/NSLookupTool/NSLookupTool.tsx https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719466454 :

@@ -7,50 +7,54 @@ import { UserGuide } from "../UserGuide/UserGuide"; import { SaveOutputToTextFile_v2 } from "../SaveOutputToFile/SaveOutputToTextFile"; import { LoadingOverlayAndCancelButton } from "../OverlayAndCancelButton/OverlayAndCancelButton";

-const title = "NsLookup"; -const description_userguide =

  • "The nslookup command is a tool used to query Domain Name System (DNS) servers and retrieve information about a specific domain or IP address." + +/**
    • Title and Description of the tool

These constants need to be moved to within the function.

In src/components/NSLookupTool/NSLookupTool.tsx https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719467150 :

@@ -7,50 +7,54 @@ import { UserGuide } from "../UserGuide/UserGuide"; import { SaveOutputToTextFile_v2 } from "../SaveOutputToFile/SaveOutputToTextFile"; import { LoadingOverlayAndCancelButton } from "../OverlayAndCancelButton/OverlayAndCancelButton";

-const title = "NsLookup"; -const description_userguide =

  • "The nslookup command is a tool used to query Domain Name System (DNS) servers and retrieve information about a specific domain or IP address." + +/**
    • Title and Description of the tool
  • */ +const title = "NSLookup"; +const description_Userguide =

Change this constant title to description. ⬇️ Suggested change

-const description_Userguide = +const description =


In src/components/NSLookupTool/NSLookupTool.tsx https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719467739 :

@@ -7,50 +7,54 @@ import { UserGuide } from "../UserGuide/UserGuide"; import { SaveOutputToTextFile_v2 } from "../SaveOutputToFile/SaveOutputToTextFile"; import { LoadingOverlayAndCancelButton } from "../OverlayAndCancelButton/OverlayAndCancelButton";

-const title = "NsLookup"; -const description_userguide =

  • "The nslookup command is a tool used to query Domain Name System (DNS) servers and retrieve information about a specific domain or IP address." + +/**
    • Title and Description of the tool
  • */ +const title = "NSLookup"; +const description_Userguide =
  • "The NSLookup command is a tool used to query Domain Name System (DNS) servers and retrieve information about a specific domain or IP address." + "This command is an essential tool for network administrators and system engineers as it can be used to troubleshoot DNS issues and gather information about DNS configurations." + "How to use NSLookUp.\n\n" + "Step 1: Enter an IP or Web URL.\n" +

Steps should be saved in their own variable const steps.

In src/components/NSLookupTool/NSLookupTool.tsx https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719470207 :

  • let form = useForm({
  • const [loading, setLoading] = useState(false); // Indication of running the process
  • const [pid, setPid] = useState(""); // Process ID
  • const [output, setOutput] = useState(""); // Output
  • const [allowSave, setAllowSave] = useState(false); // Looking whether the process can be saved
  • const [hasSaved, setHasSaved] = useState(false); // Indication of saved process

the useEffect function can go here. ⬇️ Suggested change

- +// Check if the command is available and set the state variables accordingly.

  • useEffect(() => {
  • // Check if the command is available and set the state variables accordingly.
  • checkAllCommandsAvailability(dependencies)
  • .then((isAvailable) => {
  • setIsCommandAvailable(isAvailable); // Set the command availability state
  • setOpened(!isAvailable); // Set the modal state to opened if the command is not available
  • setLoadingModal(false); // Set loading to false after the check is done
  • })
  • .catch((error) => {
  • console.error("An error occurred:", error);
  • setLoadingModal(false); // Also set loading to false in case of error
  • });
  • }, []);

In src/components/NSLookupTool/NSLookupTool.tsx https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719471432 :

  • let form = useForm({
  • const [loading, setLoading] = useState(false); // Indication of running the process
  • const [pid, setPid] = useState(""); // Process ID
  • const [output, setOutput] = useState(""); // Output
  • const [allowSave, setAllowSave] = useState(false); // Looking whether the process can be saved
  • const [hasSaved, setHasSaved] = useState(false); // Indication of saved process
  • // Form management by using Mantine

This is probably a more descriptive comment for this particular piece of code. ⬇️ Suggested change

  • // Form management by using Mantine
  • // Form Hook to handle form input.

In src/components/NSLookupTool/NSLookupTool.tsx https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719474027 :

 const clearOutput = useCallback(() => {
  • setOutput("");
  • setHasSaved(false);
  • setAllowSave(false);
  • }, [setOutput]); // Dependency on the setOutput function.
  • setOutput(""); // Clears output
  • setHasSaved(false); // Reset save status
  • setAllowSave(false); // Disable saving
  • }, [setOutput]);
 return (
     <form onSubmit={form.onSubmit((values) => onSubmit(values))}>

Can add RenderComponent and loadingModal here. ⬇️ Suggested change

  • <form onSubmit={form.onSubmit((values) => onSubmit(values))}>
  • <>
  • <RenderComponent
  • title={title}
  • description={description}
  • steps={steps}
  • tutorial={tutorial}
  • sourceLink={sourceLink}
  • {!loadingModal && (
  • <InstallationModal
  • isOpen={opened}
  • setOpened={setOpened}
  • feature_description={description}
  • dependencies={dependencies}
  • )}
  • <form onSubmit={form.onSubmit((values) => onSubmit(values))}>

In src/components/NSLookupTool/NSLookupTool.tsx https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719475163 :

 "This command is an essential tool for network administrators and system engineers as it can be used to troubleshoot DNS issues and gather information about DNS configurations." +

"How to use NSLookUp.\n\n" + "Step 1: Enter an IP or Web URL.\n" +

  • " E.g. 127.0.0.1\n\n" +
  • " E.g. 127.0.0.1\n\n" + "Step 2: View the Output block below to view the results of the Scan.";

You also need to add the tutorial and sourceLink variables.

In src/components/NSLookupTool/NSLookupTool.tsx https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719476892 :

return ( <form onSubmit={form.onSubmit((values) => onSubmit(values))}> {LoadingOverlayAndCancelButton(loading, pid)}

- {UserGuide(title, description_userguide)} + {UserGuide(title, description_Userguide)}

Can delete userguide. It is not required as part of baselining. ⬇️ Suggested change

  • {UserGuide(title, description_Userguide)}

In src/components/NSLookupTool/NSLookupTool.tsx https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719477820 :

             />

+

  • <TextInput label={"Tutorial"} {...form.getInputProps("tutorial")} />
  • {SaveOutputToTextFile_v2(output, allowSave, hasSaved, handleSaveComplete)} <Button type={"submit"}>Scan

Convention is "Start " ⬇️ Suggested change</p> <ul> <li><Button type={"submit"}>Scan</Button></li> <li><Button type={"submit"}>Start {Title}</Button></li> </ul> <hr /> <p>In src/components/NSLookupTool/NSLookupTool.tsx <a href="https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719479196">https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#discussion_r1719479196</a> :</p> <blockquote> <pre><code> "Step 2: View the Output block below to view the results of the Scan.";</code></pre> </blockquote> <p>-interface FormValues {</p> <ul> <li>ipaddress: string; +// Form Value Interface +interface FormValuesType {</li> <li>ipAddress: string; // Ip Address that needs to be looked up</li> <li> <p>tutorial: string; //Tutorial Text }</p> <p>export function NSLookup() {</p> </li> </ul> <p>I do not think you need export because you export it later. ⬇️ Suggested change</p> <p>-export function NSLookup() { +function NSLookup() {</p> <p>— Reply to this email directly, view it on GitHub <a href="https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#pullrequestreview-2242103833">https://github.com/Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740#pullrequestreview-2242103833</a>, or unsubscribe <a href="https://github.com/notifications/unsubscribe-auth/BHW54WAWVEFH4AB6SRYEERTZRWUKPAVCNFSM6AAAAABMKLHGGKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDENBSGEYDGOBTGM">https://github.com/notifications/unsubscribe-auth/BHW54WAWVEFH4AB6SRYEERTZRWUKPAVCNFSM6AAAAABMKLHGGKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDENBSGEYDGOBTGM</a> . You are receiving this because you were mentioned.Message ID: <Hardhat-Enterprises/Deakin-Detonator-Toolkit/pull/740/review/2242103833@ github.com></p> </blockquote> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/michaeljpigott"><img src="https://avatars.githubusercontent.com/u/35479648?v=4" />michaeljpigott</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <p>Hi @hathimsh, </p> <p>I am not sure what you mean - this looks like your most up-to-date pull request. </p> <p>Kind regards</p> <p>Michael</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/michaeljpigott"><img src="https://avatars.githubusercontent.com/u/35479648?v=4" />michaeljpigott</a> commented <strong> 2 months ago</strong> </div> <div class="markdown-body"> <p>Closing this pull request because it has been replaced by pull request #800 </p> </div> </div> <div class="page-bar-simple"> </div> <div class="footer"> <ul class="body"> <li>© <script> document.write(new Date().getFullYear()) </script> Githubissues.</li> <li>Githubissues is a development platform for aggregating issues.</li> </ul> </div> <script src="https://cdn.jsdelivr.net/npm/jquery@3.5.1/dist/jquery.min.js"></script> <script src="/githubissues/assets/js.js"></script> <script src="/githubissues/assets/markdown.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/highlight.min.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/languages/go.min.js"></script> <script> hljs.highlightAll(); </script> </body> </html>