Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

Adjacent lock #133

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Adjacent lock #133

wants to merge 7 commits into from

Conversation

hc00364289
Copy link
Contributor

Adjacent lock

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 1, 2021
@zlavergne zlavergne linked an issue Feb 3, 2021 that may be closed by this pull request
Copy link
Contributor

@zlavergne zlavergne left a comment

Choose a reason for hiding this comment

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

I've got a number of things that need to be addressed in the inline comments. Additionally, there's a couple other things:

  • Let's also implement this for validation. If someone locks something for validation, we want to prevent the surrounding tasks from being locked. This shouldn't be much of a change, just using the adjacent_task_lock/unlock functions when we lock for validation too
  • Make sure to remove any unnecessary comments or console logs.

@@ -207,7 +207,7 @@ class ProjectDTO(Model):
enforce_random_task_selection = BooleanType(
required=False, default=False, serialized_name="enforceRandomTaskSelection"
)

adjacent_task_lock = BooleanType(required=True, default=False, serialized_name='adjacentTaskLock')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
adjacent_task_lock = BooleanType(required=True, default=False, serialized_name='adjacentTaskLock')
prevent_adjacent_task_lock = BooleanType(required=True, default=False, serialized_name='preventAdjacentTaskLock')

Similarly to how the name "Adjacent Task Lock" is confusing, let's call it "Prevent Adjacent Task Lock" throughout the project.

@@ -136,6 +136,7 @@ class Project(db.Model):
enforce_random_task_selection = db.Column(
db.Boolean, default=False
) # Force users to edit at random to avoid mapping "easy" tasks
adjacent_task_lock = db.Column(db.Boolean, default=False) # Turn on adjacent lock
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
adjacent_task_lock = db.Column(db.Boolean, default=False) # Turn on adjacent lock
prevent_adjacent_task_lock = db.Column(db.Boolean, default=False) # Turn on adjacent lock

Same

@@ -359,6 +360,7 @@ def update(self, project_dto: ProjectDTO):
self.priority = ProjectPriority[project_dto.project_priority].value
self.default_locale = project_dto.default_locale
self.enforce_random_task_selection = project_dto.enforce_random_task_selection
self.adjacent_task_lock = project_dto.adjacent_task_lock
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.adjacent_task_lock = project_dto.adjacent_task_lock
self.prevent_adjacent_task_lock = project_dto.prevent_adjacent_task_lock

Same

@@ -969,6 +971,7 @@ def _get_project_and_base_dto(self):
self.validation_permission
).name
base_dto.enforce_random_task_selection = self.enforce_random_task_selection
base_dto.adjacent_task_lock = self.adjacent_task_lock
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
base_dto.adjacent_task_lock = self.adjacent_task_lock
base_dto.prevent_adjacent_task_lock = self.prevent_adjacent_task_lock

@@ -1163,6 +1166,11 @@ def get_project_campaigns(project_id: int):

return campaign_list

def get_project_adj_toggle(project_id: int):
""" get the value of adjacent task lock true/false for project"""
query = db.session.query(Project.adjacent_task_lock).filter(Project.id==project_id).all()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
query = db.session.query(Project.adjacent_task_lock).filter(Project.id==project_id).all()
query = db.session.query(Project.prevent_adjacent_task_lock).filter(Project.id==project_id).all()

Same

Comment on lines 32 to 43
{
label: getLabel('ADJACENT_LOCK'),
field: 'adjacentLock',
backgroundColor: TASK_COLOURS.ADJACENT_LOCK,
borderColor: '#929db3',
},
{
label: getLabel('ADJACENTLOCK'),
field: 'adjacentLock',
backgroundColor: TASK_COLOURS.ADJACENTLOCK,
borderColor: '#929db3',
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are there two here? It looks like we only use both in different parts of the code. Let's try to remove one of them and unify the rest of the code usage.

@@ -104,6 +108,7 @@ export const TasksMap = ({
if (map.getSource('tasks') === undefined) {
map.addImage('lock', lockIcon, { width: 17, height: 20, data: lockIcon });
map.addImage('redlock', redlockIcon, { width: 30, height: 30, data: redlockIcon });
// map.addImage('adjacentLock', adjLockIcon, { width: 130, height: 130, data: adjLockIcon });
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove if not necessary

Comment on lines 135 to 136
console.log('adjacent lock is', adjacentlocked);
console.log(' lock is', locked);
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove if not necessary

let taskStatusCondition = ['case'];
console.log('task status ', taskStatusCondition);
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Comment on lines 43 to 49
export const OSM_SERVER_URL =
process.env.REACT_APP_OSM_SERVER_URL || 'https://www.openstreetmap.org';
export const ID_EDITOR_URL =
process.env.REACT_APP_ID_EDITOR_URL || 'https://www.openstreetmap.org/edit?editor=id&';
export const POTLATCH2_EDITOR_URL =
process.env.REACT_APP_POTLATCH2_EDITOR_URL ||
'https://www.openstreetmap.org/edit?editor=potlatch2';
Copy link
Contributor

Choose a reason for hiding this comment

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

If these are just formatting changes, let's remove them so we avoid conflicts

@zlavergne zlavergne added the PRIORITY Work on these first label Nov 9, 2021
@zlavergne zlavergne added this to the Hi-Pri Features milestone Nov 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Component: Backend Component: Frontend PRIORITY Work on these first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjacent Task Lock
3 participants