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

Make Goal mutable and able to be updated #352

Merged
merged 4 commits into from
Nov 28, 2022
Merged

Conversation

theospears
Copy link
Collaborator

@theospears theospears commented Nov 27, 2022

Originally Goal objects were immutable and replaced on change. However, as we move towards an architecture where there are objects which keep references to goals, it makes more sense to have them be mutable and updated (and ideally with only one instance per goal). This was somewhat true before, but has become more explicit with recent changes.

Here we allow goals to be refreshed, update some controllers to use this functionality, and address some resulting concurrency issues.

Test Plan:
Smoke test running locally for a while

We wish to be able to re-initialize goal based on new data. If the new data
has values missing we should unset the appropriate values in the structure,
rather than persisting the old values. Thus we remove conditionals and instead
allow nils to be set.

Test Plan:
* Confirmed app loads and shows gallery in simulator
Goal objects now expose a way to update their values from new JSON. This
does not allow the goal ID to change (though the slug could, as beeminder
allows you to rename goals).
Ideally we would make this an actor, but lots of its methods need to be called
synchronously
@theospears theospears merged commit be5b56e into master Nov 28, 2022
@theospears theospears deleted the json-goal-can-update branch November 28, 2022 03:36
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.

1 participant