[GH-66] Improve date format selection UI. Fixes #66 #86

sanjaydemansol · 2021-08-27T04:53:55Z

Summary

Now, Hashtag of Agenda Items has two parts:

Prefix
Date Format (Dropdown of predefined items)

Testing notes

Open agenda settings for the channel and enter desired prefix and choose date format.
Create new agenda item in that group.(/agenda queue Task 1)

Output after chosen format:

Ticket Link

Fixes #66
Fixes #91

codecov · 2021-08-27T04:56:36Z

Codecov Report

Merging #86 (e407281) into master (bc0e65d) will increase coverage by 0.55%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #86      +/-   ##
==========================================
+ Coverage   24.56%   25.12%   +0.55%     
==========================================
  Files           7        7              
  Lines         403      406       +3     
==========================================
+ Hits           99      102       +3     
  Misses        287      287              
  Partials       17       17

Impacted Files	Coverage Δ
server/meeting.go	`42.16% <100.00%> (+2.16%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc0e65d...e407281. Read the comment docs.

mickmister · 2021-08-27T05:21:08Z

@sanjaydemansol Please make sure "Fixes #66" is in the description of the PR when it is submitted. Otherwise, the issue and PR will not be linked.

maisnamrajusingh

LGTM - @mickmister Might be best if you took a look at this first before reviewing #12 and #47

webapp/src/components/meeting_settings/meeting_settings.jsx

Co-authored-by: Jason Frerich <[email protected]>

dipak-demansol · 2021-09-13T13:24:55Z

Tested and failed

issue 1 :- after creating 3 agenda i deleted 2nd agenda then i create 4th agenda then again 3rd number assign to the 4th agenda so task 2 & task 3 agenda number is same.
#agenda is : Sep_14 1) Task 1
#agenda is : Sep_14 3) task 2
#agenda is : Sep_14 3) Task 3

issue 2 :- in the summary i can see the validation as "You may use underscore_. Other special characters including - , not allowed." but i created agenda using those special character as "#agenda is : !@#$%^&*()_+=?><:"{}2021_9_14 8) test message".

issue 3 :- there should be a validation if user click on the save button without selecting the "Meeting Day", because currently without selecting the meeting day if user create the agenda then user is getting this error as
System 6:37 PM(Only visible to you)
Error calculating hashtags. Check the meeting settings for this channel.

improvement :- it would be great if you add colour to the date same as agenda colour so it will be easy to read the date.

sanjaydemansol · 2021-09-14T02:44:26Z

Tested and failed

issue 1 :- after creating 3 agenda i deleted 2nd agenda then i create 4th agenda then again 3rd number assign to the 4th agenda so task 2 & task 3 agenda number is same.
#agenda is : Sep_14 1) Task 1
#agenda is : Sep_14 3) task 2
#agenda is : Sep_14 3) Task 3

issue 2 :- in the summary i can see the validation as "You may use underscore_. Other special characters including - , not allowed." but i created agenda using those special character as "#agenda is : !@#$%^&*()_+=?><:"{}2021_9_14 8) test message".

issue 3 :- there should be a validation if user click on the save button without selecting the "Meeting Day", because currently without selecting the meeting day if user create the agenda then user is getting this error as
System 6:37 PM(Only visible to you)
Error calculating hashtags. Check the meeting settings for this channel.

improvement :- it would be great if you add colour to the date same as agenda colour so it will be easy to read the date.

Hi @dipak-demansol, welcome to the Mattermost community,
Issue 1: Maybe you are testing functionality expected from #88 in this PR, kindly use code from #88 to test this functionality.
issue 2: Its intended for backwards compatibility.
issue3: Its out of scope for this PR.

dipak-demansol · 2021-09-14T06:17:48Z

Tested and failed
issue 1 :- after creating 3 agenda i deleted 2nd agenda then i create 4th agenda then again 3rd number assign to the 4th agenda so task 2 & task 3 agenda number is same.
#agenda is : Sep_14 1) Task 1
#agenda is : Sep_14 3) task 2
#agenda is : Sep_14 3) Task 3
issue 2 :- in the summary i can see the validation as "You may use underscore_. Other special characters including - , not allowed." but i created agenda using those special character as "#agenda is : !@#$%^&*()_+=?><:"{}2021_9_14 8) test message".
issue 3 :- there should be a validation if user click on the save button without selecting the "Meeting Day", because currently without selecting the meeting day if user create the agenda then user is getting this error as
System 6:37 PM(Only visible to you)
Error calculating hashtags. Check the meeting settings for this channel.
improvement :- it would be great if you add colour to the date same as agenda colour so it will be easy to read the date.

Hi @dipak-demansol, welcome to the Mattermost community,
Issue 1: Maybe you are testing functionality expected from #88 in this PR, kindly use code from #88 to test this functionality.
issue 2: Its intended for backwards compatibility.
issue3: Its out of scope for this PR.

@sanjaydemansol ok, so as per this PR, I'm able to Create agenda using the different Date Format and also able to update the Prefix so i can say Its Pass.

mickmister · 2021-10-01T20:01:24Z

@DHaussermann This PR has been marked by @dipak-demansol as QA Review Done, but I'm concerned about the UX of this feature.

@matthewbirtch Can you or another designer take a look at this and the linked issue #66 to see if this is going in the right direction UX-wise? The goal of this addition is to give the user some guidance (really a fixed set of options) on creating an agenda hashtag format, rather than having them learn the rules of the agenda hashtag formatting.

mickmister · 2021-10-01T20:02:15Z

@sanjaydemansol What is the purpose of moving away from using dashes, and using underscores instead? At the moment, I think creating agenda hashtags with dashes is more common.

mickmister

Thanks for the updates on the PR @sanjaydemansol 👍 I've left some requests and questions on the code

mickmister · 2021-10-01T20:02:42Z

server/meeting.go

-	HashtagFormat string         `json:"hashtagFormat"` // Default: {ChannelName}-Jan02
+	HashtagFormat string         `json:"hashtagFormat"` // Default: {ChannelName}-Jan-2


Why did the default change here?

Hashtags with - were returning no results for me, after some digging found out:

ref
2/5, our plugin should be database agnostic but not sure if I should include these changes in this PR.
@mickmister

I'm not able to reproduce this issue using postgress using #foo-02. Is there anything more specific case that causes issues?

Case was same, but issue appeared randomly.

mickmister · 2021-10-01T20:04:29Z

webapp/src/components/meeting_settings/meeting_settings.jsx

+                                    name='format'
+                                    value={this.state.dateFormat}
+                                    onChange={this.handleDateFormat}
+                                    style={{height: '35px', border: '1px solid #ced4da'}}


Can this be done using an scss file rather than using style directly? If you have questions on this, please ask on the Mattermost community server

Sure, will move all css to scss.

mickmister · 2021-10-01T20:08:08Z

webapp/src/components/meeting_settings/meeting_settings.jsx

-                                href='https://godoc.org/time#pkg-constants'
-                            >{'Go time package.'}</a>
-                            {' Embed a date by surrounding what January 2, 2006 would look like with double curly braces, i.e. {{Jan02}}'}
+                        <div style={{display: 'flex'}}>


What does setting this value do in this case? It doesn't look like there are any alignItems, justifyContent, etc. rules being applied in the element's children.

Actually, this expands items to fill available free space or shrinks them to prevent overflow.

mickmister · 2021-10-01T20:14:11Z

webapp/src/components/meeting_settings/meeting_settings.jsx

+            const splitResult = this.props.meeting.hashtagFormat.split('{{');// we know, date Format is preceded by {{
+            const hashtagPrefix = splitResult[0];
+            const dateFormat = splitResult[1].substring(0, splitResult[1].length - 2); // remove trailing }}


Can we trust that this.props.meeting.hashtagFormat is a correctly formatted string? i.e. it has {{ and }}

Also if it's an existing agenda setting that is made before this PR, do we know that dateFormat will be equal to one of the available options?

For the new ones, yes. Since we are assigning value from controlled values.
no, if any unknown value is there. 1st value from the list is used.

mickmister · 2021-10-01T20:16:47Z

webapp/src/components/meeting_settings/meeting_settings.jsx

    handleHashtagChange = (e) => {
        this.setState({
-            hashtag: e.target.value,
+            hashtagPrefix: e.target.value,
        });
    }

+    handleDateFormat = (event) => {
+        this.setState({
+            dateFormat: event.target.value,
+        });
+    };


Can we be consistent and use e in both of these cases, instead of using event here?

sure, will do that.

mickmister · 2021-10-01T20:21:20Z

webapp/src/components/meeting_settings/meeting_settings.jsx

+                                style={{padding: '5px'}}
+                            >
+                                <label className='control-label'>{'Date Format'}</label>
+                                <br/>


Are we able to accomplish this with css instead of using <br/>?

@mickmister removed <br/> tag and there was no affect on design.

mickmister · 2021-10-01T20:22:20Z

webapp/src/components/meeting_settings/meeting_settings.jsx

            weekdays: [1],
+            dateFormat: '1-2',


What does 1-2 mean?

Great catch, seems like I missed this while separating commits, will fix this.

mickmister · 2021-10-01T20:23:46Z

webapp/src/components/meeting_settings/meeting_settings.jsx

@@ -17,8 +17,9 @@ export default class MeetingSettingsModal extends React.PureComponent {
        super(props);

        this.state = {
-            hashtag: '{{Jan02}}',
+            hashtagPrefix: 'Prefix',


Do we have a way for the user to define their own date format that is used, like the feature currently works?

no, as per issue. IMO it is end goal to limit that.

mickmister · 2021-10-01T20:24:44Z

webapp/src/components/meeting_settings/meeting_settings.jsx

+                                    <option value='Jan 2'>{'month_day'}</option>
+                                    <option value='2 Jan'>{'day_month'}</option>
+                                    <option value='1 2'>{'month_day'}</option>
+                                    <option value='2 1'>{'day_month'}</option>
+                                    <option value='2006 1 2'>{'year_month_day'}</option>


Why do the values have spaces instead of underscores?

because underscore has special significance for go time package, which we use. Hence we have this workaround for swapping _ with .

mickmister · 2021-10-01T20:26:47Z

server/meeting_test.go

 				}},
-			want:    "#QA-" + assertNextWeekdayDate(time.Monday, true).Format("January 02 2006"),
+			want:    "#QA_" + strings.ReplaceAll(assertNextWeekdayDate(time.Monday, true).Format("January 02 2006"), " ", "_"),


It seems like a big drive of certain changes involve avoiding dashes and using underscores instead, which is an opinionated approach that is not backwards compatible with existing agenda settings. Why is this change required?

#86 (comment)

mickmister · 2021-10-05T00:13:54Z

Tested and failed
issue 1 :- after creating 3 agenda i deleted 2nd agenda then i create 4th agenda then again 3rd number assign to the 4th agenda so task 2 & task 3 agenda number is same.
#agenda is : Sep_14 1) Task 1
#agenda is : Sep_14 3) task 2
#agenda is : Sep_14 3) Task 3
issue 2 :- in the summary i can see the validation as "You may use underscore_. Other special characters including - , not allowed." but i created agenda using those special character as "#agenda is : !@#$%^&*()_+=?><:"{}2021_9_14 8) test message".
issue 3 :- there should be a validation if user click on the save button without selecting the "Meeting Day", because currently without selecting the meeting day if user create the agenda then user is getting this error as
System 6:37 PM(Only visible to you)
Error calculating hashtags. Check the meeting settings for this channel.
improvement :- it would be great if you add colour to the date same as agenda colour so it will be easy to read the date.

Issue 2: Its intended for backwards compatibility

@sanjaydemansol Can you explain the considerations that have been made for backwards compatibility? What compatibility issues are taken into account in the PR?

@sanjaydemansol @dipak-demansol Can you please provide test cases for testing the backwards compatibility. Then the test cases can be documented and @sanjaydemansol can write appropriate unit tests for the different cases.

Issue3: Its out of scope for this PR.

If it is a serious issue captured that is out of scope for the PR, please make sure that either the person that found the bug or the person that pointed out that it's out of scope creates a ticket to make sure the issue is fixed. @sanjaydemansol If you would like @dipak-demansol to create it (maybe he has more context of the full problem), you can ask him to create it, otherwise you can ask someone else, or you can create the ticket.

The main point is to make it so the bug doesn't fall through the cracks. You can use GitHub's "Reference in new issue" feature that allows you to easily copy a PR comment into a new issue:

dipak-demansol · 2021-10-05T06:19:10Z

Tested and failed
issue 1 :- after creating 3 agenda i deleted 2nd agenda then i create 4th agenda then again 3rd number assign to the 4th agenda so task 2 & task 3 agenda number is same.
#agenda is : Sep_14 1) Task 1
#agenda is : Sep_14 3) task 2
#agenda is : Sep_14 3) Task 3
issue 2 :- in the summary i can see the validation as "You may use underscore_. Other special characters including - , not allowed." but i created agenda using those special character as "#agenda is : !@#$%^&*()_+=?><:"{}2021_9_14 8) test message".
issue 3 :- there should be a validation if user click on the save button without selecting the "Meeting Day", because currently without selecting the meeting day if user create the agenda then user is getting this error as
System 6:37 PM(Only visible to you)
Error calculating hashtags. Check the meeting settings for this channel.
improvement :- it would be great if you add colour to the date same as agenda colour so it will be easy to read the date.

Issue 2: Its intended for backwards compatibility

@sanjaydemansol Can you explain the considerations that have been made for backwards compatibility? What compatibility issues are taken into account in the PR?

@sanjaydemansol @dipak-demansol Can you please provide test cases for testing the backwards compatibility. Then the test cases can be documented and @sanjaydemansol can write appropriate unit tests for the different cases.

Issue3: Its out of scope for this PR.

If it is a serious issue captured that is out of scope for the PR, please make sure that either the person that found the bug or the person that pointed out that it's out of scope creates a ticket to make sure the issue is fixed. @sanjaydemansol If you would like @dipak-demansol to create it (maybe he has more context of the full problem), you can ask him to create it, otherwise you can ask someone else, or you can create the ticket.

The main point is to make it so the bug doesn't fall through the cracks. You can use GitHub's "Reference in new issue" feature that allows you to easily copy a PR comment into a new issue:

@mickmister hi, Thanks for the reply, i will test this PR again, If the issue regenerate again then i will report it and if its out of scope then i create new issue. also i will add the test cases.

hanzei

What is the impact on existing agenda setting using - in there hashtag?

hanzei · 2021-10-05T09:46:39Z

server/meeting.go

-	HashtagFormat string         `json:"hashtagFormat"` // Default: {ChannelName}-Jan02
+	HashtagFormat string         `json:"hashtagFormat"` // Default: {ChannelName}-Jan-2


I'm not able to reproduce this issue using postgress using #foo-02. Is there anything more specific case that causes issues?

mattermod · 2022-02-28T01:01:13Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @aspleenic

… the Data/Text

guna-demansol · 2022-03-15T07:14:10Z

Screen.Recording.2022-03-15.at.12.41.48.PM.mov

Please review

mickmister · 2022-04-27T18:38:46Z

/update-branch

mattermod · 2022-04-27T18:38:48Z

We don't have permissions to update this PR, please contact the submitter to apply the update.

mickmister · 2022-04-27T18:40:13Z

@guna-demansol Do you mind pushing another commit to trigger CI to create a build for this? CircleCI deletes them after one month. Thank you!

mickmister · 2022-04-27T18:41:17Z

@DHaussermann This will require regression testing on existing agenda settings, to make sure upgrading to this version of the plugin doesn't break existing settings, especially agenda settings that use dashes, as there is some string replacement going on in the PR - to _, but I'm not entirely sure what all it affects at the moment, which is why I'm bringing this up.

mickmister · 2022-04-27T18:42:05Z

Once a build is available, I will upload the build to a cloud server for UX to test

hanzei · 2023-02-21T12:08:11Z

/update-branch

mattermost-build · 2023-02-21T12:08:13Z

We don't have permissions to update this PR, please contact the submitter to apply the update.

Improve date format selection UI mattermost-community#66

3395a30

sanjaydemansol requested a review from jfrerich as a code owner August 27, 2021 04:53

mickmister linked an issue Aug 27, 2021 that may be closed by this pull request

Improve date format selection UI #66

Open

hanzei added the 2: Dev Review Requires review by a core committer label Aug 27, 2021

jfrerich requested a review from mickmister August 31, 2021 14:38

jfrerich added the 3: QA Review Requires review by a QA tester label Aug 31, 2021

sanjaydemansol requested review from sibasankarnayak and maisnamrajusingh September 2, 2021 04:28

maisnamrajusingh approved these changes Sep 3, 2021

View reviewed changes

sibasankarnayak approved these changes Sep 3, 2021

View reviewed changes

jfrerich mentioned this pull request Sep 9, 2021

[GH-47] [FIX] Agenda item number should get updated if the items of the agenda are deleted #88

Closed

jfrerich suggested changes Sep 10, 2021

View reviewed changes

webapp/src/components/meeting_settings/meeting_settings.jsx Outdated Show resolved Hide resolved

webapp/src/components/meeting_settings/meeting_settings.jsx Outdated Show resolved Hide resolved

webapp/src/components/meeting_settings/meeting_settings.jsx Outdated Show resolved Hide resolved

sanjaydemansol and others added 3 commits September 13, 2021 07:34

Update webapp/src/components/meeting_settings/meeting_settings.jsx

69ef21a

Co-authored-by: Jason Frerich <[email protected]>

Update webapp/src/components/meeting_settings/meeting_settings.jsx

066a6ef

Co-authored-by: Jason Frerich <[email protected]>

Update webapp/src/components/meeting_settings/meeting_settings.jsx

177bcb9

Co-authored-by: Jason Frerich <[email protected]>

sanjaydemansol requested a review from jfrerich September 14, 2021 06:09

dipak-demansol added QA Review Done PR has been approved by QA and removed 3: QA Review Requires review by a QA tester labels Sep 23, 2021

mickmister requested changes Oct 1, 2021

View reviewed changes

mickmister requested a review from hanzei October 1, 2021 20:27

hanzei reviewed Oct 5, 2021

View reviewed changes

jfrerich removed their request for review February 10, 2022 18:02

hanzei removed the Awaiting Submitter Action Blocked on the author label Feb 15, 2022

maisnamrajusingh unassigned sanjaydemansol Feb 17, 2022

mattermod added the Lifecycle/1:stale label Feb 28, 2022

GuNa Skr and others added 5 commits March 11, 2022 21:21

mattermost-community#66 in the Dark theme The User is not able to see…

dbca404

… the Data/Text

Merge remote-tracking branch 'upstream/master' into mmt-66

eec7368

mattermost-community#66 dark theme issue CircleCi error fixes

080b4d8

mattermost-community#66 dark theme issue CircleCi error fixes

13865c0

mattermost-community#66 dark theme issue CircleCi error fixes

2edb8c8

mattermost-community#66 removing <br/> tag as per comment

d6dad99

dipak-demansol requested a review from matthewbirtch April 8, 2022 12:02

maisnamrajusingh added 2 commits April 28, 2022 18:57

Update LICENSE

0156f0b

Update LICENSE

e407281

mickmister removed their request for review August 4, 2022 15:20

hanzei added Lifecycle/3:orphaned and removed 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester Lifecycle/1:stale labels Feb 21, 2023

hanzei removed this from the v0.3.0 milestone Feb 21, 2023

hanzei closed this Feb 21, 2023

		HashtagFormat string `json:"hashtagFormat"` // Default: {ChannelName}-Jan02
		HashtagFormat string `json:"hashtagFormat"` // Default: {ChannelName}-Jan-2

[GH-66] Improve date format selection UI. Fixes #66 #86

[GH-66] Improve date format selection UI. Fixes #66 #86

Conversation

sanjaydemansol commented Aug 27, 2021 • edited Loading

Summary

Testing notes

Ticket Link

codecov bot commented Aug 27, 2021 • edited Loading

Codecov Report

mickmister commented Aug 27, 2021

maisnamrajusingh left a comment

Choose a reason for hiding this comment

dipak-demansol commented Sep 13, 2021

sanjaydemansol commented Sep 14, 2021

dipak-demansol commented Sep 14, 2021

mickmister commented Oct 1, 2021

mickmister commented Oct 1, 2021

mickmister left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanjaydemansol Oct 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mickmister commented Oct 5, 2021

dipak-demansol commented Oct 5, 2021

hanzei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattermod commented Feb 28, 2022

guna-demansol commented Mar 15, 2022

mickmister commented Apr 27, 2022

mattermod commented Apr 27, 2022

mickmister commented Apr 27, 2022

mickmister commented Apr 27, 2022 • edited Loading

mickmister commented Apr 27, 2022

hanzei commented Feb 21, 2023

mattermost-build commented Feb 21, 2023

sanjaydemansol commented Aug 27, 2021 •

edited

Loading

codecov bot commented Aug 27, 2021 •

edited

Loading

sanjaydemansol Oct 5, 2021 •

edited

Loading

mickmister commented Apr 27, 2022 •

edited

Loading