Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override refactoring and additional functionnalities #159

Closed

Conversation

avouspierre
Copy link
Contributor

The PR includes a large refactoring of the swift part of override/profile functions :

  • Override is stored in override core data, including history
  • Override preset is stored in overridepreset core data
  • Add the display of the override in main graph
  • add the upload of override as a exercice in Nightscout - Fix [feature request] Display profiles/overrides in Nightscout  #145
  • improve the management of indefinate override / stop of indefinate override
  • modify the code to respect the Ivan’s patterns of the app :
  • Use of swiftInject (dependency injection) with the use of protocol class in the code
  • Use of MVP principes, in particular not use of direct coredata in view class
  • Use of a proxy model class between coredata and the app to manage changes of core data
  • Use of the pattern of observe to refresh data/view/uploads
  • add a core data unit tests allowing to add tests for coredata with a in-memory datastore for tests.
  • test for overrideStorage available

This PR do NOT change the logic with oref and the interface of override informations in oref. This PR do NOT require a update of trio-oref code.

TODO : Changes the shortcuts after merging with PR #144 and add watch for overrides.

override display override in NS
Simulator Screenshot - iPhone 15 - 2024-05-07 at 21 43 34 CleanShot 2024-05-07 at 21 44 16

@avouspierre
Copy link
Contributor Author

I created a new PR to be able to merge with the current dev version. Unable to do with the previous PR.

@dnzxy
Copy link
Contributor

dnzxy commented May 11, 2024

Have not thoroughly reviewed the PR, but just on a brief skim:

  • Little typo in several places, OverrideProfil needs an e so it's Profile
  • Please properly catch and handle errors instead of just optionally trying them. I saw that you (mostly) gracefully unwrap or guard / if let… these optionals, but simply opting for nil results hides error until they may show up in a very different layer of the app. A concise error log would be more helpful imho.

Please pull in latest dev so this can be checked out and tested on top of latest dev :)

Overall, great work.

@avouspierre
Copy link
Contributor Author

Please pull in latest dev so this can be checked out and tested on top of latest dev :)

Thanks @dnzxy. I updated the branch with the last version of the dev, did few tests in simulator but not updated with your proposals. Will do later in the week-end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature request] Display profiles/overrides in Nightscout
2 participants