google / peoplemath Goto Github PK
View Code? Open in Web Editor NEWWeb application for team planning
License: Apache License 2.0
Web application for team planning
License: Apache License 2.0
It would be useful to be able to group and tag objectives, in order to provide views that aggregate objectives together.
For example, objectives could be grouped into projects or products, so it's possible to see how much of a team's resources are being spent on each, and get a sense of the stack ranking of these aggregates.
Also, tags would be useful to provide highlighting of objectives with certain themes.
Grouping would be a mutually-exclusive concept: each objective could be a member of at most one group of each "group type" (e.g. "project").
Tagging, by contrast, would be a many-to-many concept: each objective could have multiple tags, and each tag could be applied to multiple objectives.
Currently, it is possible to add multiple users with the same ID. This should be prevented, as it causes unexpected consequences throughout the UI.
Right now build_appengine.sh
passes --prod
to ng build
, and to change that you have to edit the script. It would be good to have a simpler way of doing this.
Now the summary report has multiple panes, a table of contents and a set of internal anchor links would be very useful.
Create new package "controllers" and create 2 controllers files in it: team.go period.go. Inside controllers we can put handlers like Get() Post() Put() - i think it could be cool.
What do you think about it?
@amdw reply
Splitting the team-related and period-related controllers into separate files sounds reasonable to me if we can make it work. There may be some code shared between those controllers, but we can cross that bridge when we come to it.
Currently, reordering objectives is done using up and down buttons. This makes it hard to move an objective a long way (e.g. from the top to the bottom). Also, with a long objective name, the buttons wrap and overlap in an ugly way.
There should be a better way of doing this, e.g. drag-and-drop.
This is quite a destructive operation, so careful verification would be needed before actually doing it (especially if there are actual objectives in the bucket). But it's still something that needs to be done sometimes.
For teams split across multiple locations or timezones, it may be helpful to have that information in the people table, so for example you can sort by it.
Currently, PeopleMath has no support for authenticating users, or for restricting read or write access to any part of the application to certain groups of users.
I suspect many teams who might consider deploying this application outside Google would find this useful. Few would want the entire world to be able to read, let alone modify, all their data.
Many users would probably benefit from a more sophisticated access control scheme, where different teams have different groups of users who are able to read and write data associated with those teams.
Ideally the application should support a pluggable authentication scheme. The Google App Engine documentation suggests several options, but the implementation should not assume the application is running in Google App Engine; authentication should be abstracted such that you can drop in another mechanism if you want to without changing anything else.
Authorization should also be pluggable. The simplest implementation on App Engine would probably be to represent user groups as objects in Cloud Datastore, on a global and per-team basis (so a user would have access to a team if they are in either the relevant (read or write) global group, or the team-specific group). However, again, it should be possible to substitute another implementation for those not using App Engine. Some users, for example, may want to integrate with a role-based access control system inside their organisation.
There are some places in the UI where we determine whether the current user has edit permissions, based on the contents of the teamPermissions
. Examples:
peoplemath/src/app/period/period.component.ts
Line 284 in f56a7b6
This duplicates logic on the server side. For example, we need to look at environment.requireAuth
to decide whether the server will even try to do any authentication at all.
There are places where we do make this kind of determination based on server-side logic, such as canAddTeam
on the team list:
peoplemath/backend/controllers/team.go
Line 70 in 5d7a3c7
We should use this technique consistently.
This may require backwards-incompatible changes to the JSON API, namely adding a wrapper object that contains the permissions and the team itself. But this is fine, as the backend is tightly coupled to the frontend at the moment (there is no mobile app whose versions can skew). We already made such a change to the team list.
Angular 10 is now out. We should upgrade.
https://update.angular.io/#9.0:10.0l3
With this version, strict mode is now the default for new apps. We should try to opt into as many of these strict settings as possible.
The design of the dialog to assign people to objectives needs a bit of a rethink.
A few ideas:
If authentication is turned off, the following code does not work:
peoplemath/src/app/period/period.component.ts
Line 283 in 5d7a3c7
The allow array will be empty in this case, but that doesn't mean the user doesn't have permissions, because the whole auth feature is switched off.
Sometimes objectives belong together in the stack rank, e.g. when it makes no sense to complete one without the other during the period, but users prefer not to merge them into a single objective for clarity or brevity.
In such cases, the ability to group objectives together can be useful. E.g. if you want to move them in a block within the stack rank, it's inconvenient to do so by dragging them one at a time.
It's not immediately clear to me how this need could be addressed in the UI.
There are occasions when it would be useful to have certain types of rich formatting, such as links or bold/italic text, in objectives and/or their notes.
This could be accomplished by supporting a markdown subset or some similar syntax. There is almost certainly already a suitable Javascript library for this.
Currently, objectives show in grey if they are unassigned. This is intended to emphasize that these objectives will not be worked on during the period.
An objective with zero as its resource estimate is one which does not require any effort in the period; such an objective can never have any assignments and will always show in grey.
It would be good to distinguish these two types of objectives. Perhaps an even lighter shade of grey for zero-estimate objectives?
This runtime is available now:
https://cloud.google.com/appengine/docs/standard/go/runtime#go-1.13
This will require a warning if the person has any assignments.
Currently, if you want to rename a group or tag, the only way to do it is to open each objective with that group or tag and update the values one-by-one.
It would be good to have a way to consistently rename a group or tag across all objectives with a single action.
It appears to delete the objective from the old bucket without adding it to the new bucket. In other words, it is currently functionally equivalent to delete. :(
We would benefit from a continuous build for this project, which:
npx ng build --prod
) to ensure there are no errors (and ideally warnings)npx ng test --browsers ChromeHeadless
)cd backend && go test ./...
)Ideally this would run automatically on every pull request.
Keyboard shortcuts could make several user journeys quite a bit easier, especially in the Period view when editing. A particular example would be adding a new objective, but editing, deleting and reordering objectives would also be worth considering.
When you try to print either the period or period-summary view, you only get a single page, regardless of how much content there is.
The page you get aligns with the current browser view - in other words, if you're scrolled to the top of the page, you get what you'd normally expect to be the first page, but if you're scrolled further down, you'll get a single-page "window" on the middle of the document.
A little searching suggests this seems to be quite a common issue in Angular applications, and might be possible to fix with a few CSS changes. For example: angular/components#8378
Printing would be particularly valuable for the period summary view, as it would allow you to take a snapshot of that page, say in PDF format. However, hopefully fixing the underlying issue will make printing "just work" for all views.
Currently, there is a "show objectives" toggle in the group summary pane(s) in the period summary view, to allow the specific objectives to be displayed underneath each group.
It would be possible to use a tree for this instead, so that the user could expand individual groups to see the objectives. Angular Material has a tree component:
https://material.angular.io/components/tree/overview
The advantage of this would be that the user could easily open and close individual groups of interest. Currently, it's easy to lose your scroll position when you turn on the "show objectives" toggle, especially if there are a lot of groups and/or objectives.
The disadvantage of using the standard Angular Material tree component would be that the view would become much less information-dense: each tree node is much taller than an individual list item.
Currently, renaming group A to B in the breakdown for group type X will rename all groups named A to B. It should only rename groups of type X.
Entries in the "assignments by person" section overlap each other once they get too long.
Sometimes objectives belong together in the stack rank, e.g. when it makes no sense to complete one without the other during the period, but users prefer not to merge them into a single objective for clarity or brevity.
In such cases, the ability to combine adjacent objectives into a single "block" would be useful. E.g. if you want to move them together within the stack rank, it's inconvenient to do so by dragging them one at a time.
It's not immediately clear to me how this need could be addressed in the UI.
Currently, when you run the app with existing teams in storage that don't have any permissions, you get an exception in the frontend here, because team.teamPermissions.write.allow
is null
:
peoplemath/src/app/period/period.component.ts
Line 283 in 5d7a3c7
We need to scrub such periods here, to ensure the slice of matchers is never nil
:
Some users would like to be able to associate notes and action items with their period models. Sometimes this information may be more confidential than the model itself, such as notes on team members' skills and abilities which led to the choice of assignments.
The simplest way to implement this would be to allow a link to be specified, that would create a "notes" or "action items" button in the period view that would open that URL in a new window. That link could be for example a Google Doc, which would provide access to all the features of G Suite, such as access controls and comments.
An alternative would be to add a "notes" field to each objective, and allow notes to be switched on and off in the view. The problem with this is that it would be another view "mode" that needs testing, and could lead to a more cluttered UI. The need for additional access controls would also make things more complex, both in the UI and the backend.
It is also not necessarily the case that all notes map directly to objectives: they may apply to groups of objectives or to the model as a whole. At the moment I favour adding a simple "notes" link to the period model as an easy-to-implement compromise.
Sometimes you want to move an objective from one bucket to another. It would be good to have a better way than deleting it from one bucket and then adding it back to another.
Currently, period models can be edited by anyone able to make an HTTP request to the application backend. The application performs no authentication or authorization checks of its own, relying on a proxy server to apply such checks before requests reach it.
This makes it cumbersome to apply more sophisticated access controls, such as "group A may read and write periods for team X, while group B may only read them; group C may read and write periods for team Y".
The best solution I currently see would be to add fields like "readACL" and "readWriteACL" to the Team entity, indicating the lists of users (or groups) that can perform those actions to entities belonging to that team. Then, the application itself would need to perform authentication on all requests for those entities, checking that the requesting user is authorised before proceeding to serve the request.
To protect users from locking themselves out accidentally, an empty ACL (the default) should probably mean access is unrestricted. The UI should also probably warn the user if they are about to perform an action that would restrict their own access in some way.
The specific authentication mechanism used should be "pluggable", with the details hidden behind a Go interface, similar to the way the data storage mechanism is currently.
The ability to control access to the list of teams, and to the ability to add new teams, may also be useful.
It should be possible to override the target of the "improve this" link via an environment property. This is so that teams can point it to internal resources with information about that specific instance.
Some objectives can be completed with little or no assistance from other teams, but others cannot.
It would be good to think about whether there are some simple ways to model and show the extent to which an objective depends on other teams allocating people over the same period. Possibly just a boolean for "high" versus "low" external dependencies, displayed as a slightly different background colour?
It would be useful to see the cumulative sum of the effort estimates for each objective, in descending priority order within each bucket.
This would give a sense of which objectives it's likely to be possible to work on during the period, based on the resource limits for each bucket, without actually assigning the objectives.
The numbers could be shown on the right of each bucket, perhaps colour coded to show when the cumulative sum rises above the resource limit.
The Angular Material purple "C" badge currently used to mark committed objectives is offset to the top left of each objective. This is how badges currently work in Angular Material.
Because of this, badges can overlap each other unless each objective is in a box with a lot of padding, such as a mat-list-item
. This happens for example in the breakdowns by person, group and tag in the main period view, which is quite ugly. Because of this, a text "[C]" was used in the summary view.
What I'd prefer is to use a consistent marker for this throughout the app, something that is smaller and in line with the text. I think something like the badge in Bootstrap would work well:
https://getbootstrap.com/docs/4.4/components/badge/
I don't believe Angular Material supports anything like this right now, so would have to give some consideration to how best to implement it.
It should be possible to mark an objective as either "aspirational" or "committed", and have it labelled as such in the UI.
It would make data flow within the application easier to reason about if all component inputs were immutable.
This would also allow change detection optimisations such as OnPush
to be used (article).
The best way to enforce this would be to use actual immutable datatypes (such as the ones provided by Immutable) for the types of the component inputs.
For now this entity contains store and defaultTimeout. defaultTimeout - I suggest to put it in "storage" package. StorageService can be placed globally as a variable in "storage" package. This improvement should give us "low-coupling", because storage and REST API actions are different abstractions in a programm.
Reply from @amdw
I'm open to suggestions on this, but I'm not generally a big fan of global variables. The fact that StorageService is behind an interface is already intended to provide loose coupling between the HTTP server and the storage implementation, and the storeTimeout (which determines how long the server is prepared to wait for the store to return a value) seems more a a concern of the HTTP server than the storage service. Having said that, I'm happy to look at what you have in mind.
After setting up the continuous build, I noticed a few unrepeatable failures in the Angular tests. In a typical failed build, a single test would fail, with a stack trace like this:
Chrome Headless 85.0.4183.102 (Linux x86_64) ERROR
An error was thrown in afterAll
TypeError: this.storage.updatePeriod is not a function
at <Jasmine>
at PeriodComponent.performSave (http://localhost:9876/_karma_webpack_/src/app/period/period.component.ts:333:18)
at SafeSubscriber._next (http://localhost:9876/_karma_webpack_/src/app/period/period.component.ts:67:75)
at SafeSubscriber.__tryOrUnsub (http://localhost:9876/_karma_webpack_/node_modules/rxjs/_esm2015/internal/Subscriber.js:183:1)
at SafeSubscriber.next (http://localhost:9876/_karma_webpack_/node_modules/rxjs/_esm2015/internal/Subscriber.js:122:1)
at Subscriber._next (http://localhost:9876/_karma_webpack_/node_modules/rxjs/_esm2015/internal/Subscriber.js:72:1)
at Subscriber.next (http://localhost:9876/_karma_webpack_/node_modules/rxjs/_esm2015/internal/Subscriber.js:49:1)
at DebounceTimeSubscriber.debouncedNext (http://localhost:9876/_karma_webpack_/node_modules/rxjs/_esm2015/internal/operators/debounceTime.js:40:1)
at AsyncAction.dispatchNext (http://localhost:9876/_karma_webpack_/node_modules/rxjs/_esm2015/internal/operators/debounceTime.js:53:1)
at AsyncAction._execute (http://localhost:9876/_karma_webpack_/node_modules/rxjs/_esm2015/internal/scheduler/AsyncAction.js:51:1)
at AsyncAction.execute (http://localhost:9876/_karma_webpack_/node_modules/rxjs/_esm2015/internal/scheduler/AsyncAction.js:39:1)
I'm not sure exactly what's going on here, but this feels like it might be a race condition of some kind.
The check of the LastUpdateUUID
currently performed by the ensureNoConcurrentMod
function in main.go
should be performed in the same database transaction as the actual write of the Period
. The transaction should be committed only if the UUID on the incoming object matches the one in the database.
Under the current implementation, the check is performed outside the database write transaction. That means there is still a race condition whereby two updates to the same Period
take place concurrently, both having a correct LastUpdateUUID
. If both updates pass the ensureNoConcurrentMod
check before either is written to the database, both write transactions will succeed, and both update requests should receive a successful HTTP response code, when in fact one write has stomped over the other one.
The same is true of the checks in ensureTeamExistence
and ensurePeriodExistence
as well, though race conditions here are not as harmful, as these checks are repeated (in the current google_cds_store.go
) inside the database transaction. The same thing could be done for the UUID check (i.e. verifying it both in main.go
and a database transaction), though it would be better to simply perform all the checks once in the database layer where possible, and make this part of the semantics of the StorageService
.
This would likely require modifying the StorageService
interface to return custom error codes, so that the different types of error can be distinguished in main.go
and suitable HTTP status codes returned to the client in the different cases.
Currently, in the period view, it's necessary to scroll to the top to access the edit button and reorder toggle. Ideally these would be accessible from any scroll position.
These functions should probably live in HTTP controllers, not in the Cloud Datastore code.
This would mean the scrubbing would automatically happen regardless of the underlying storage implementation, and they would be easier to test as well.
This assumes users will have access to the server logs, which won't be true in all cases.
We should consider other ideas, e.g. putting a UUID in the error message which the user can supply to the administrator of their instance, making it easy for them to find the right log entry.
On the main period view, I see signs of unnecessary change detections making the app a little sluggish when there are lots of objectives.
For example, I see the "C" badges on committed objectives flashing when you press a key in the edit dialogs, and typing into text fields feels a bit sluggish.
There should be no need for change detection to run in these cases. We need to profile to check what's going on, then consider techniques for reducing the triggering of change detection and/or reducing DOM manipulation.
In some cases it may be useful to have additional resource units for a period, convertible to or from the primary unit using a fixed conversion factor.
For example, the main unit for a period might be person weeks, but it might also be interesting in places like the summary report to see figures in headcount, assuming one headcount is assumed to be a fixed number of person weeks.
A couple of times I've accidentally dismissed the snackbar that tells you a concurrent edit has been made.
This error should be made more noticeable - perhaps a modal that can only be dismissed by clicking a certain button, and/or maybe pressing the escape key?
It should prompt the user to copy any unsaved work and then reload the page.
You have relations in your models. Why did you choose NoSQL? Dont you want to create SQL store type just to try? I can do it also. I have created today docker env #42 so I can add PostgreSQL container with adminer.
@amdw reply
Regarding the choice of data store: a NoSQL database seemed like a good fit because the Period data structure is quite deeply nested (not having to do joins potentially brings some performance gains), and we have quite simple querying needs. The lack of schema makes it very easy to add new fields - you just modify the Go structs and everything just works.
The nice thing about Cloud Datastore specifically is that there is quite a generous free tier, meaning that you can run small instances on App Engine without paying anything, which is not the case even for the smallest SQL database (as Cloud SQL instances require a dedicated VM).
That said, I can well imagine that some users would prefer to use a SQL data store, and I'm quite happy to accept a new version of the StorageService that uses a SQL database instead, perhaps using Gorm or similar. The only thing is that I'm not likely to use it myself, so I can't promise to invest time in maintaining it.
Docs https://www.gorillatoolkit.org/ .
This is flexible tool, this will allow us to create a separate route for each method (GET, POST) so we dont need to use if/else condition.
@amdw reply
Sounds good - the Gorilla libraries seem quite well-regarded, so if this simplifies the code then I'm all for it!
This code is likely to be common to any Auth
implementation which wants to perform actual permission checks. It therefore doesn't really belong in Firebase-specific code:
peoplemath/backend/auth/firebase.go
Line 68 in 2fcd365
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.