This is a list of all comments for LEAHI-APPLICATION-LDT-1616-1. Review Summary: No summary ---------------------------------------- File: configurations/Parameters/DataList.conf Revision Comment by Behrouz NematiPour on 16 September 2025, 14:06 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24306 * Please put both the K and Ca ranges under the category: [Acid Concentrate Ranges] rename them like: [Acid Concentrate Ranges] Acid_Concentrate_Potassium_Def Acid_Concentrate_Potassium_Min Acid_Concentrate_Potassium_Max Acid_Concentrate_Potassium_Res Acid_Concentrate_Calcuim_Def Acid_Concentrate_Calcuim_Min Acid_Concentrate_Calcuim_Max Acid_Concentrate_Calcuim_Res Reply by Nicholas Ramirez on 16 September 2025, 14:20 > updated Reply by Behrouz NematiPour on 16 September 2025, 23:14 > RESOLVED ---------------------------------------- File: sources/gui/qml/pages/pretreatment/create/PreTreatmentCreateContent.qml Revision Comment by Behrouz NematiPour on 16 September 2025, 14:33 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24312 Could you use update instead of sync, for consistency, since it has not been used anywhere for that purpose, and also sync is mainly used for cloud data communications? For all the accuracies All, Synchronize, sync, ..... Reply by Nicholas Ramirez on 16 September 2025, 14:52 > terminology updated from sync to refresh Reply by Behrouz NematiPour on 17 September 2025, 11:34 > Thanks, that works too. > RESOLVED. Revision Comment by Behrouz NematiPour on 22 September 2025, 15:30 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24422 Why is this changed to the properties, what about the others? does it make the code out of sync between functions? Reply by Nicholas Ramirez on 23 September 2025, 11:29 > I am using the *set from the VALUESET macro to sync the > isActive of the 2 instances of the component Reply by Behrouz NematiPour on 23 September 2025, 12:43 > understood, but you have now two varables/propery for this > purpose the one with _ in qml and the one with set in C++. > how do they get in sync? Reply by Nicholas Ramirez on 23 September 2025, 13:21 > so the isActive is the internal property of ValueAdjuster > and it is binded to the c++ property of <*>set. Similar > to how we binded the value to the c++ property Reply by Behrouz NematiPour on 24 September 2025, 10:03 > RESOLVED. Revision Comment by Behrouz NematiPour on 16 September 2025, 14:42 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24317 To note for later, I was going to add a comment in the previous CR, The new naming canEdit() is a lot clearer than 'enableEditing()'. Reply by Nicholas Ramirez on 16 September 2025, 14:52 > i agree Reply by Behrouz NematiPour on 16 September 2025, 16:53 > RESOLVED. Revision Comment by Behrouz NematiPour on 16 September 2025, 14:52 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24319 move this to the Variables to make sur the same is typed always and use the code completion qml checks. keep the qsTr in the Variables.qml definitions. Or define in here (same qml) if not used anywhere else. Reply by Nicholas Ramirez on 16 September 2025, 15:33 > updated and defined variables in this file as its only used > here Reply by Behrouz NematiPour on 17 September 2025, 11:33 > RESOLVED. Revision Comment by Behrouz NematiPour on 16 September 2025, 14:58 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24321 *TBDL: Let's discuss more later.* Can you explain this block? Why it calls the doCancell() regardless of the menu selection, shouldn't it be only for the Edit Rx? And what is the actualIndex++? Reply by Nicholas Ramirez on 16 September 2025, 15:34 > When we choose something from the 3 dot prescription menu we > first check if parameters are validated. If so then we do the > doCancel to notify FW and then go back to the re-validate > state which sets everything to be editable for any option > chosen. > > For the actualIndex i see the confusion but that was to > offset teh index when the 'Edit Rx' is removed from the > model. I updated to compare the text of the model activated > so we no longer have to monitor the index. This works > regardless of what model is present Reply by Behrouz NematiPour on 16 September 2025, 16:50 > Thanks for the clarification. > Let me dig deeper during the execution review, and we'll > discuss it then. > > TBDL: Let's discuss more later. Reply by Behrouz NematiPour on 24 September 2025, 09:54 > RESOLVED Revision Comment by Behrouz NematiPour on 16 September 2025, 16:32 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24329 *TBDL: Let's discuss more later.* Please use only - default - value instead of the - defaultValue -> default - actualValue -> value Reply by Nicholas Ramirez on 17 September 2025, 08:04 > i cant update the property name to default because its a > reserved keyword in qml. I also cant change actualValue to > value because we already have a value property that serves as > the main value for this component. Actual value is intended > only for the refresh to update with the "actual" value set in > c++. Reply by Behrouz NematiPour on 17 September 2025, 13:57 > Why can't we use value for the same purpose as the > actualValue? Reply by Nicholas Ramirez on 18 September 2025, 10:46 > reason adding actualValue is to achieve synchronization > of both create Rx and the Rx popup. actualValue is only > used in the refresh() method inisde of ValueAdjsuter.qml. > To update the other instance of PreTreatmentCreateContent > with the set vTreatmentCreate. when its updated on > the other one. Reply by Nicholas Ramirez on 22 September 2025, 10:27 > updated per meeting last week Reply by Behrouz NematiPour on 24 September 2025, 09:41 > RESOLVED Revision Comment by Nicholas Ramirez on 19 September 2025, 15:32 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24395 Remove this and update c++ vCreate onRelease Reply by Nicholas Ramirez on 22 September 2025, 09:55 > updated Reply by Behrouz NematiPour on 24 September 2025, 10:16 > RESOLVED ---------------------------------------- File: sources/gui/qml/components/HeaderBar.qml Revision Comment by Nicholas Ramirez on 19 September 2025, 15:11 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24394 do not show show when in create Rx screen Reply by Nicholas Ramirez on 22 September 2025, 11:50 > updated to not show when on create Rx screen Reply by Behrouz NematiPour on 23 September 2025, 12:44 > RESOLVED. Revision Comment by Behrouz NematiPour on 17 September 2025, 13:28 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24355 *Execution Review:* 1 - The Rx button should only be visible/work when the op_mode is one of the following (or not the other ones). - 4 - MODE_TPAR - 5 - MODE_PRET - 6 - MODE_TREA - 7 - MODE_POST 2 - And when the Create Rx screen itself is visible/active. 3 - A request: - Can you populate the currently added K, Ca from the previous Acit Concentrate addition by the pen icon? - And if the set K, Ca in the dialog is the same as previously added, notify the user of the duplication, same as what is done for the constant ones which come from the settings. Reply by Nicholas Ramirez on 17 September 2025, 15:59 > updated and added this behavior Reply by Behrouz NematiPour on 18 September 2025, 15:27 > RESOLVED ---------------------------------------- File: sources/gui/qml/compounds/ValueAdjuster.qml Revision Comment by Behrouz NematiPour on 17 September 2025, 11:37 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24351 Please do not use "do" for properties. Those are intended for public slots. *For properties use "is", "has", "can", ...* Even active was better to be as "isActive" since it is bool. Reply by Nicholas Ramirez on 17 September 2025, 12:12 > change to canRefresh and isActive Reply by Behrouz NematiPour on 18 September 2025, 15:25 > Thanks > RESOLVED. Revision Comment by Behrouz NematiPour on 17 September 2025, 11:45 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24352 Use "can" instead of "allow" to be consistent. Reply by Nicholas Ramirez on 17 September 2025, 12:11 > updated allowOff to canAllowOff Reply by Behrouz NematiPour on 17 September 2025, 13:42 > Please just "canOFF" is sufficient. Reply by Nicholas Ramirez on 18 September 2025, 07:56 > made the variable name update Reply by Behrouz NematiPour on 18 September 2025, 11:01 > RESOLVE. Revision Comment by Behrouz NematiPour on 23 September 2025, 13:14 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24429 If not isActive, then canIncrement? Reply by Nicholas Ramirez on 23 September 2025, 13:19 > yes when not active you can press increment or decrement > which will then show the value and set value to the default > value Reply by Behrouz NematiPour on 24 September 2025, 09:57 > RESOLVED Revision Comment by Behrouz NematiPour on 23 September 2025, 13:15 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24430 same? Reply by Nicholas Ramirez on 23 September 2025, 13:19 > same Reply by Behrouz NematiPour on 24 September 2025, 09:57 > RESOLVED ---------------------------------------- File: sources/gui/qml/dialogs/AcidConcentrateAdjustment.qml Revision Comment by Behrouz NematiPour on 17 September 2025, 17:45 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24362 - If only the dialog becomes visible, we should do that, and when it is closed, you should not call the function. - The naming is confusing, since it does not check; it only fills the fields with the values. *Execution Review* - When you populate the values from the previously set K and Ca, a notification appears indicating a duplicate entry. Reply by Nicholas Ramirez on 18 September 2025, 07:50 > Name updated to doPopulateAcidConcentrate() and to only call > when visible and clear notification when not. Reply by Behrouz NematiPour on 18 September 2025, 15:31 > Thanks. > RESOLVED. Revision Comment by Behrouz NematiPour on 17 September 2025, 18:06 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24363 When the dialog is invisible, the notification shall be cleared for the next time. Reply by Nicholas Ramirez on 18 September 2025, 07:51 > updated Reply by Behrouz NematiPour on 18 September 2025, 11:01 > RESOLVED. Revision Comment by Behrouz NematiPour on 17 September 2025, 18:11 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24364 1 - The following properties belong to the same caller object: - vTreatmentCreate.acidConcentrateSet - vTreatmentCreate.acidConcentrate Therefore, the object already has it. Why do you send that back to it? 2 - You have the parameters in the following properties of the same object. Why do you work with the index to get the text and split it? - acidConcentratePotassium - acidConcentrateCalcium Reply by Nicholas Ramirez on 18 September 2025, 07:34 > 1-They come from different caller objects. The caller > containing the acid concentrate QStringList is > VTreatmentRanges and the set and selected index come from > VTreatmentCreate. > > 2- Inside VTreatmentRanges the acidConcentratePotassium and > acidConcentrateCalcium are RANGESET(). They get the ranges > from the .conf file only so i could not use those to store > the set value. Reply by Behrouz NematiPour on 18 September 2025, 15:32 > My bad, > Thanks for the clarification. > Misread the views. > RESOLVED. Revision Comment by Behrouz NematiPour on 22 September 2025, 11:15 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24410 - Change the vSet to vActive to make it clearer. - Please use .isActive = vActive after addressing the above comment. Reply by Nicholas Ramirez on 22 September 2025, 11:35 > updated Reply by Behrouz NematiPour on 24 September 2025, 09:54 > RESOLVED ---------------------------------------- File: sources/gui/qml/main.qml Revision Comment by Behrouz NematiPour on 16 September 2025, 22:35 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24332 *TBDL: Let's discuss more later.* Move to the PreTreatmentCreateStack.qml Think of these types of things as modules. If you have the CreateTreatment as a module, the Stack manager file shall take care of screens and their state, and not the main.qml. Is there any reason they are called here and not on the section that has been removed from the Create Stack? Reply by Nicholas Ramirez on 17 September 2025, 07:55 > Moved them here to consolidate the slots. Since I updated the > clear call to c++ and signal back to the shared > PreTreatmentCreateContent.qml so both instances can clear > simultaneously, the clear was not dependent in being inside > the CreateTreatment. Reply by Behrouz NematiPour on 17 September 2025, 11:30 > Related comment. > > TBDL: Let's discuss more later. Reply by Nicholas Ramirez on 22 September 2025, 10:33 > reverted the clear Reply by Behrouz NematiPour on 22 September 2025, 15:25 > RESOLVED ---------------------------------------- File: sources/gui/qml/pages/MainStack.qml Revision Comment by Behrouz NematiPour on 16 September 2025, 16:15 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24325 *TBDL: Let's discuss more later.* Why is this removed? Reply by Nicholas Ramirez on 16 September 2025, 16:23 > this is handled by vTreatmentCreate.doClear() to synchronize > the clearing on both the pretreatment create rx and popup. > The call has been consolodated and moved to the > onStandbyChanged slot in main.qml Reply by Behrouz NematiPour on 16 September 2025, 22:40 > The logic here is to have a hierarchical, clear call that > the mainstack on an event calls the related stack clear, > and that stack calls each screen clear. > If possible, keep the logic; if not, let's discuss. > in the following comment: > http://devapps.diality.us:8060/cru/LEAHI-APPLICATION-LDT-1616-1#CFR-76254 > A clear() function should be included in the > CreateTxContent screen, which the CreateStack calls. Reply by Behrouz NematiPour on 17 September 2025, 11:30 > related comment. > > TBDL: Let's discuss more later. Reply by Nicholas Ramirez on 22 September 2025, 10:28 > updated and reverted this Reply by Behrouz NematiPour on 22 September 2025, 15:26 > RESOLVED. ---------------------------------------- File: sources/gui/qml/pages/pretreatment/create/PreTreatmentCreate.qml Revision Comment by Behrouz NematiPour on 17 September 2025, 13:47 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24357 *Execution Review:* Close the dialog when you receive the *accepted* confirm response from the FW. Reply by Nicholas Ramirez on 18 September 2025, 07:54 > updated per offline discussion. No confirm response. Updated > to close when entering MODE_PRET Reply by Behrouz NematiPour on 18 September 2025, 15:29 > Thanks > RESOLVED. ---------------------------------------- File: sources/gui/qml/pages/pretreatment/create/PreTreatmentCreateStack.qml Revision Comment by Behrouz NematiPour on 16 September 2025, 16:14 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24324 *TBDL: Let's discuss more later.* The clear functions in the stack are to call all the clear functions of the screens managed by the stack. Any reason why it was removed from here? Reply by Nicholas Ramirez on 16 September 2025, 16:22 > this is handled by vTreatmentCreate.doClear() to synchronize > the clearing on both the pretreatment create rx and popup. > The call has been updated and moved to the onStandbyChanged > slot in main.qml Reply by Behrouz NematiPour on 17 September 2025, 11:32 > Related comment. > > TBDL: Let's discuss more later. Reply by Nicholas Ramirez on 22 September 2025, 10:28 > reverted this Reply by Behrouz NematiPour on 22 September 2025, 15:27 > RESOLVED. ---------------------------------------- File: sources/view/VTreatmentCreate.cpp Revision Comment by Behrouz NematiPour on 16 September 2025, 22:46 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24334 *TBDL: Let's discuss more later.* I do not understand the intent of this function call that only emits a signal. This is the call graph that I do not see as necessary (correct me if I am wrong): PreTreatmentCreateContent.qml (PreTreatmentCreateContent.Clear) -> vTreatmentCreate.doClear() -> VTreatmentCreate::doClear() -> emit VTreatmentCreate::didClear() -> vTreatmentCreate.onDidClear() -> PreTreatmentCreateContent (_root.clear() ) From the PreTreatmentCreateContent.qml it goes into C++ and comes back in the same caller (PreTreatmentCreateContent), why? I bleieve we should follow the same stacks to screens clear calls explained in the following comment: http://devapps.diality.us:8060/cru/LEAHI-APPLICATION-LDT-1616-1#CFR-76254 Reply by Nicholas Ramirez on 17 September 2025, 08:15 > The intent is to synchronize. If the user clears either the > popup Rx or the pretreatment create Rx then only that > instance was clearing before. Not both. So now when we > initiate the clear (when back to standby "onStandbyChanged" > or when the "Clear All" option is selected in prescription ) > the order is (Q_INVOKABLE call ) vTreatmentCreate.doClear() > -> (c++ signal) emit didClear() -> (slot in qml) onDidClear() > -> (QML method) _root.clear(). The _root.clear() is inside > the PreTreatmentCreateContent so all instances of it will get > cleared at the same time synchronizing them and this removes > that stack dependency linking all those clear methods down to > the same _root.clear Reply by Behrouz NematiPour on 17 September 2025, 10:56 > In which case do you think we should clear only one > instance of CreateRx, and there should be only one instance > of the CreateRxContent? > The one clear function should be evident in the content. > I am not sure this implementation helps the intent. > > TBDL: Let's discuss more later. Reply by Nicholas Ramirez on 22 September 2025, 10:32 > reverted this Reply by Behrouz NematiPour on 22 September 2025, 11:22 > Thanks. > RESOLVED. ---------------------------------------- File: sources/view/VTreatmentCreate.h Revision Comment by Behrouz NematiPour on 16 September 2025, 23:02 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24335 From what I can see, these two signals are no longer in use. Please ensure that you notify the user of rejection from FW. And move forward only when FW accepts the request. Reply by Nicholas Ramirez on 17 September 2025, 08:10 > didValidationPass() is still being used and the slot in in > PreTreatmentCreateContent. The didValidationFail() is no > longer being used. The user notified of rejection by that > failing fw components border changing colors and the confirm > button is disabled until they try again. Do we want an > additional message notifying the failure? Reply by Behrouz NematiPour on 17 September 2025, 11:24 > Thanks for the clarification. > No need for any extra notification, that would be redundant > then. > Although it is not used, since its pair (pass) is used, > let's keep the fail signal. > RESOLVED. ---------------------------------------- File: sources/view/hd/data/VTreatmentRanges.cpp Revision Comment by Behrouz NematiPour on 22 September 2025, 15:25 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24418 It does not seem like the bool parameter of this signal is useful, remove the bool and just emit the signal. Reply by Nicholas Ramirez on 23 September 2025, 08:57 > removed parameter Reply by Behrouz NematiPour on 23 September 2025, 12:49 > RESOLVED. ---------------------------------------- File: sources/view/hd/data/VTreatmentRanges.h Revision Comment by Behrouz NematiPour on 16 September 2025, 14:13 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24308 please rename to: acidConcentratePotassium acidConcentrateCalcium Reply by Nicholas Ramirez on 16 September 2025, 14:20 > updated Reply by Behrouz NematiPour on 17 September 2025, 11:33 > Thanks > RESOLVED. ---------------------------------------- File: sources/gui/qml/components/MainMenu.qml Revision Comment by Behrouz NematiPour on 16 September 2025, 14:11 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1#c24307 Since we have the underlying highlighter, why do we need this? Reply by Nicholas Ramirez on 16 September 2025, 14:21 > added as a request from caryn. Reply by Behrouz NematiPour on 16 September 2025, 16:17 > It is not necessary, and will be redundant. > Please share extra requests before modifications. Reply by Nicholas Ramirez on 17 September 2025, 09:42 > reverted this back Reply by Behrouz NematiPour on 17 September 2025, 10:53 > RESOLVED. --- ID: LEAHI-APPLICATION-LDT-1616-1 https://devapps.diality.us/cru/LEAHI-APPLICATION-LDT-1616-1 Title: LEAHI-APPLICATION-LDT-1616_HeaderBar - SW - 01 - Prescription - R&I Statement of Objectives: State: Closed Summary: Author: Nicholas Ramirez Moderator: Nicholas Ramirez Reviewers: (5 active, 1 completed*) Behrouz NematiPour (*) Tiffany Mejia Dara Navaei Vendor - TEL - Sivvanarayana Kurapati Daniel Ho Stephen Quong