[FEATURE REQUEST] Add a member to a space: Set permissions and expiration date#4764
[FEATURE REQUEST] Add a member to a space: Set permissions and expiration date#4764
Conversation
joragua
force-pushed
the
feature/add_a_member_to_a_space
branch
from
4a42584 to
b4a9d4e
Compare
| Enhancement: Add a member to a space | ||
|
|
||
| A new option to add members has been added to the space members | ||
| list, visible only to users with the appropriate permissions. |
There was a problem hiding this comment.
My proposal:
A new option to add members to a space has been added. It will be only visible for users with proper permissions.
| calendar.get(Calendar.MONTH), | ||
| calendar.get(Calendar.DAY_OF_MONTH) | ||
| ).apply { | ||
| datePicker.minDate = calendar.timeInMillis |
There was a problem hiding this comment.
question: here, you get the current date to prevent selecting a date in the past. But, is it today allowed? It'd be a good idea to give at least one day (not expiring before tomorrow)
| calendar.set(Calendar.MILLISECOND, 999) | ||
| val formatter = SimpleDateFormat(DisplayUtils.DATE_FORMAT_ISO, Locale.ROOT) | ||
| formatter.timeZone = TimeZone.getTimeZone("UTC") | ||
| val isoExpirationDate = formatter.format(calendar.time) |
There was a problem hiding this comment.
Will the date include Z to avoid problems with date/time changes?
|
|
||
| return ((oldItem.id == newItem.id) && (oldItem.displayName == newItem.displayName) && (oldItem.surname == newItem.surname)) | ||
| } | ||
| } |
There was a problem hiding this comment.
This class is pretty similar to the SpaceMembersDiffUtils developed in the previous PR. Just a difference in the areContentsTheSame fun. Any way to refactor and prevent duplicated code?
| val listOfMembersFiltered = spaceMembers.members.sortedWith(compareByDescending<SpaceMember> { | ||
| member -> roles.indexOfFirst { it.id in member.roles } }.thenBy { member -> member.displayName }) | ||
| val diffCallback = SpaceMembersDiffUtil(this.members, listOfMembersFiltered) | ||
| val diffResult = DiffUtil.calculateDiff(diffCallback) | ||
| this.members = listOfMembersFiltered | ||
| diffResult.dispatchUpdatesTo(this) |
There was a problem hiding this comment.
i don't catch this new block replacing the "sorting by role and displayname". Why that?
| android:background="@android:color/darker_gray" | ||
| android:alpha="0.5"/> | ||
|
|
||
| </LinearLayout> |
There was a problem hiding this comment.
I miss accessibility properties here
| android:layout_height="wrap_content" | ||
| android:layout_marginStart="@dimen/standard_half_margin" | ||
| android:focusable="true" | ||
| android:clickable="true"/> |
There was a problem hiding this comment.
Is that needed? i mean, could be a RadioButton not clickable?
| android:background="@android:color/darker_gray" | ||
| android:alpha="0.5"/> | ||
|
|
||
| </LinearLayout> |
| override fun addMember( | ||
| spaceId: String, | ||
| memberId: String, | ||
| memberType: String, |
There was a problem hiding this comment.
is that memberType user or group? in that case, it might be better to set as an Enum than a string, since the number of possibilities is scoped (2 in that case)
| private const val MEMBER_ID_BODY_PARAM = "objectId" | ||
| private const val ROLES_BODY_PARAM = "roles" | ||
| } | ||
|
|
3 participants
Related Issues
App: #4740
ReleaseNotesViewModel.ktcreating a newReleaseNote()with String resources (if required)QA