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

Task/DES-2001: Project links as observables (subscribe to project-user data). #67

Merged
merged 30 commits into from
Jan 20, 2022

Conversation

duckonomy
Copy link
Contributor

@duckonomy duckonomy commented Jul 6, 2021

Overview:

PR Status:

  • Ready.
  • Work in Progress.
  • Hold.

Related Jira tickets:

Summary of Changes:

When saving/linking the map to a project, ensure that an observable project is also created.

Testing Steps:

  1. Save a map to a project and ensure that users are synced.(test with Task/DES-2001: Project links as observables (subscribe to project-user data). geoapi#65)

UI Photos:

Notes:

@duckonomy duckonomy marked this pull request as ready for review July 7, 2021 14:20
Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. 👍 I liked your ideas about changes to the modal that we discussed yesterday 💯

src/app/models/models.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can recheck when the backend PR is good to go but just wanted to leave a note of one things i saw 👍

@duckonomy
Copy link
Contributor Author

duckonomy commented Sep 3, 2021

Changes

  • Refactored create project modal to clarify underlying functionality
  • Disallows modification of existing linked observable projects.
  • Removed user add in users-panel (Will further prune add-user related functionality in another PR. Ticket)
  • Removed Unused removed unused routes

New testing steps

Test with the related geoapi branch TACC-Cloud/geoapi#65

  1. Ensure that all the creation and linking scenarios work.
  • Creating map without any linkage.
  • Creating a non-project map with just a file.
  • Creating a non-project map as an observable.
  • Creating a project map that syncs only users.
  • Creating a project map that syncs users and content.
  • Overwriting an existing link (only when saved to non-project systems/non-observable systems)
  1. Ensure that only maps linked to project systems can see users.
  2. Ensure that maps not linked to project systems cannot see the save link map button.

@duckonomy
Copy link
Contributor Author

Screenshot from 2021-12-22 14-26-03

Newest changes address Tracy and Ellen's comments on usability.

  1. By default, name is map name rather than map UUID (and automatically filled in when typing name).
  2. All maps must be saved to a location for it to be created.
  3. Clarify observable project option in the create project modal.
  4. Indicate where we are saving to.
  5. Remove "Save as" functionality for now
  6. Prevent user for creating existing file
  7. Assume that the user always syncs users when saving to projects.

@duckonomy duckonomy removed the request for review from reptillicus January 7, 2022 15:08
Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. 👍 Just a few comments to consider.

Sorry again for the late review 🙏 .

I think the only thing that really needs to be considered is the updating of map metadata (like. map name) and what we should do when that happens. Maybe things can be simpler as we aren't supporting "save as" anymore.

Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💯

@duckonomy duckonomy merged commit 0601027 into master Jan 20, 2022
@taoteg taoteg deleted the task/DES-2001-project-links-as-observables branch July 6, 2023 18:21
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.

2 participants