opendev/storyboard
Code Issues Proposed changes

Merge "Rationalise worklist ordering code"

Browse Source
This commit is contained in:
Zuul 2019-01-16 01:38:05 +00:00 committed by Gerrit Code Review
commit 601cb38607
2 changed files with 72 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
storyboard/api/v1/worklists.py
View File

@ -434,7 +434,7 @@ class ItemsSubcontroller(rest.RestController):
raise exc.NotFound(_("Item %s refers to a non-existent task or " raise exc.NotFound(_("Item %s refers to a non-existent task or "
"story.") % item_id) "story.") % item_id)
worklists_api.add_item( card = worklists_api.add_item(
id, item_id, item_type, list_position, id, item_id, item_type, list_position,
current_user=request.current_user_id) current_user=request.current_user_id)
@ -443,13 +443,12 @@ class ItemsSubcontroller(rest.RestController):
"item_id": item_id, "item_id": item_id,
"item_title": item.title, "item_title": item.title,
"item_type": item_type, "item_type": item_type,
"position": list_position "position": card.list_position
} }
events_api.worklist_contents_changed_event(id, user_id, added=added) events_api.worklist_contents_changed_event(id, user_id, added=added)
return wmodels.WorklistItem.from_db_model( return wmodels.WorklistItem.from_db_model(card)
worklists_api.get_item_at_position(id, list_position))
@decorators.db_exceptions @decorators.db_exceptions
@secure(checks.authenticated) @secure(checks.authenticated)
@ -511,7 +510,7 @@ class ItemsSubcontroller(rest.RestController):
if list_id != card.list_id and list_id is not None: if list_id != card.list_id and list_id is not None:
new['worklist_id'] = list_id new['worklist_id'] = list_id
worklists_api.move_item(id, item_id, list_position, list_id) worklists_api.move_item(item_id, list_position, list_id)
if display_due_date is not None: if display_due_date is not None:
if display_due_date == -1: if display_due_date == -1:
@ -557,8 +556,8 @@ class ItemsSubcontroller(rest.RestController):
if card is None: if card is None:
raise exc.NotFound(_("Item %s seems to have already been deleted," raise exc.NotFound(_("Item %s seems to have already been deleted,"
" try refreshing your page.") % item_id) " try refreshing your page.") % item_id)
worklists_api.update_item(item_id, {'archived': True})
worklists_api.normalize_positions(worklist) worklists_api.archive_item(item_id)
item = None item = None
if card.item_type == 'story': if card.item_type == 'story':

137
storyboard/db/api/worklists.py
View File

@ -216,16 +216,19 @@ def add_item(worklist_id, item_id, item_type, list_position,
if worklist is None: if worklist is None:
raise exc.NotFound(_("Worklist %s not found") % worklist_id) raise exc.NotFound(_("Worklist %s not found") % worklist_id)
# If the target position is "outside" the list, override it
if list_position > worklist.items.count():
list_position = worklist.items.count()
# Check if this item has an archived card in this worklist to restore # Check if this item has an archived card in this worklist to restore
archived = get_item_by_item_id( archived = get_item_by_item_id(
worklist, item_type, item_id, archived=True) worklist, item_type, item_id, archived=True)
if archived: if archived:
update = { update_item(archived.id, {'archived': False})
'archived': False, # Move the newly unarchived card into position, and move other cards
'list_position': list_position # to compensate for the move
} move_item(archived.id, list_position)
api_base.entity_update(models.WorklistItem, archived.id, update) return archived
return worklist
# If this worklist is a lane, check if the item has an archived card # If this worklist is a lane, check if the item has an archived card
# somewhere in the board to restore # somewhere in the board to restore
@ -233,13 +236,11 @@ def add_item(worklist_id, item_id, item_type, list_position,
board = boards.get_from_lane(worklist) board = boards.get_from_lane(worklist)
archived = boards.get_card(board, item_type, item_id, archived=True) archived = boards.get_card(board, item_type, item_id, archived=True)
if archived: if archived:
update = { update_item(archived.id, {'archived': False})
'archived': False, # Move the newly unarchived card into position, and move other
'list_id': worklist_id, # cards to compensate for the move
'list_position': list_position move_item(archived.id, list_position, new_list_id=worklist_id)
} return archived
api_base.entity_update(models.WorklistItem, archived.id, update)
return worklist
# Create a new card # Create a new card
if item_type == 'story': if item_type == 'story':
@ -254,20 +255,23 @@ def add_item(worklist_id, item_id, item_type, list_position,
raise exc.NotFound(_("%(type)s %(id)s not found") % raise exc.NotFound(_("%(type)s %(id)s not found") %
{'type': item_type, 'id': item_id}) {'type': item_type, 'id': item_id})
item_dict = { card_dict = {
'list_id': worklist_id, 'list_id': worklist_id,
'item_id': item_id, 'item_id': item_id,
'item_type': item_type, 'item_type': item_type,
'list_position': list_position 'list_position': 99999 # Initialise the card "outside" the list
} }
worklist_item = api_base.entity_create(models.WorklistItem, item_dict) card = api_base.entity_create(models.WorklistItem, card_dict)
# Move the card into position, and move other cards to compensate
card = move_item(card.id, list_position)
if worklist.items is None: if worklist.items is None:
worklist.items = [worklist_item] worklist.items = [card]
else: else:
worklist.items.append(worklist_item) worklist.items.append(card)
return worklist return card
def get_item_by_id(item_id, session=None): def get_item_by_id(item_id, session=None):
@ -295,67 +299,58 @@ def get_item_by_item_id(worklist, item_type, item_id, archived):
return query.first() return query.first()
def normalize_positions(worklist): def move_item(item_id, new_pos, new_list_id=None):
for item in worklist.items:
if item.archived:
item.list_position = 99999
items = worklist.items.order_by(
models.WorklistItem.list_position)
for position, item in enumerate(items):
if not item.archived:
item.list_position = position
def move_item(worklist_id, item_id, list_position, list_id=None):
session = api_base.get_session() session = api_base.get_session()
with session.begin(subtransactions=True): with session.begin(subtransactions=True):
item = get_item_by_id(item_id, session) modified_card = get_item_by_id(item_id, session)
old_pos = item.list_position old_pos = modified_card.list_position
item.list_position = list_position
if old_pos == list_position and list_id == item.list_id: # If the item hasn't actually moved, we don't need to move it.
return if (old_pos == new_pos and
(new_list_id == modified_card.list_id or
new_list_id is None)):
return modified_card
old_list = _worklist_get(item.list_id) # "old_list" is the list the item is moving from, and "new_list" is
# the list the item is moving to. In some cases (a simple reordering,
# or a card being archived) these are the same list, but separate
# variables exist to keep the reordering code simple.
old_list = _worklist_get(modified_card.list_id)
new_list = old_list
if new_list_id is not None:
new_list = _worklist_get(new_list_id)
if list_id is not None and list_id != item.list_id: # First decrement the position of everything past the old position
# Item has moved from one list into a different one. # by one, "removing" the item from the list. Archived items are
# Move the item and clean up the positions. # ignored, since they are not really part of the ordering and are
new_list = _worklist_get(list_id) # all stored with a position of 99999.
old_list.items.remove(item) for card in old_list.items:
old_list.items = old_list.items.order_by( if card.list_position > old_pos and not card.archived:
models.WorklistItem.list_position) card.list_position -= 1
for list_item in old_list.items[old_pos:]:
list_item.list_position -= 1
normalize_positions(old_list)
new_list.items = new_list.items.order_by( # Next, increment the position of everything at or past the new
models.WorklistItem.list_position) # position by one, creating "space" for the item. Archived items are
for list_item in new_list.items[list_position:]: # ignored again.
list_item.list_position += 1 for card in new_list.items:
new_list.items.append(item) if card.list_position >= new_pos and not card.archived:
normalize_positions(new_list) card.list_position += 1
else:
# Item has changed position in the list.
# Update the position of every item between the original
# position and the final position.
old_list.items = old_list.items.order_by(
models.WorklistItem.list_position)
if old_pos > list_position:
direction = 'down'
modified = old_list.items[list_position:old_pos + 1]
else:
direction = 'up'
modified = old_list.items[old_pos:list_position + 1]
for list_item in modified: # Finally, update the position of the item we're actually moving
if direction == 'up' and list_item != item: modified_card.list_position = new_pos
list_item.list_position -= 1 if new_list_id is not None:
elif direction == 'down' and list_item != item: modified_card.list_id = new_list_id
list_item.list_position += 1
normalize_positions(old_list) return modified_card
def archive_item(item_id):
update_item(item_id, {'archived': True})
# Archived items are moved to the distant "bottom" of worklists. Calling
# move_item neatly updates the positions of other items in the list to
# compensate for the archived item.
move_item(item_id, 99999)
def update_item(item_id, updated): def update_item(item_id, updated):