Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GH-32- Fix issue with agenda list numbering #44

Merged
merged 5 commits into from
Jun 25, 2020
Merged

GH-32- Fix issue with agenda list numbering #44

merged 5 commits into from
Jun 25, 2020

Conversation

marianunez
Copy link
Contributor

Summary

Fixed issue with numbering posts in agenda list by changing search posts to use new SearchPostsInTeamForUser api.

Ticket Link

GH-32

Related Pull Requests

Mattermost Server: mattermost/mattermost#14807

@marianunez marianunez requested a review from jfrerich as a code owner June 12, 2020 22:17
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2020 •
edited by codecov bot
Loading

Codecov Report

Merging #44 into master will decrease coverage by 0.13%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
- Coverage   18.42%   18.28%   -0.14%     
==========================================
  Files           6        6              
  Lines         266      268       +2     
==========================================
  Hits           49       49              
- Misses        206      208       +2     
  Partials       11       11
Impacted Files Coverage Δ
server/command.go 0.00% <0.00%> (ø)

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 aa1ac21...3d1db00. Read the comment docs.

@marianunez marianunez requested a review from lieut-data June 12, 2020 22:54
@marianunez marianunez mentioned this pull request Jun 12, 2020
Merged
lieut-data
lieut-data approved these changes Jun 12, 2020
View reviewed changes
Copy link
Contributor

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

Nice!!

@marianunez marianunez added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Jun 12, 2020
@hanzei hanzei added this to the v0.1 milestone Jun 15, 2020
jfrerich
jfrerich approved these changes Jun 15, 2020
View reviewed changes
Copy link
Contributor

@jfrerich jfrerich left a comment

Choose a reason for hiding this comment

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

Sweet! Thanks @marianunez!

@jfrerich jfrerich removed the 2: Dev Review Requires review by a core committer label Jun 15, 2020
@jfrerich jfrerich requested a review from DHaussermann June 15, 2020 15:19
@hanzei hanzei added the Awaiting Submitter Action Blocked on the author label Jun 24, 2020
@hanzei hanzei requested review from prapti and removed request for DHaussermann June 24, 2020 09:54
@hanzei
Copy link
Contributor

hanzei commented Jun 24, 2020

@prapti Given that you tested mattermost/mattermost#14807 could you also test this PR?

@hanzei hanzei removed the Awaiting Submitter Action Blocked on the author label Jun 24, 2020
prapti
prapti suggested changes Jun 25, 2020
View reviewed changes
Copy link

@prapti prapti left a comment

Choose a reason for hiding this comment

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

The sequential numbering for agenda works great now!
The issue I'm seeing is when the first item of the agenda gets deleted or the hashtag gets edited in the post the numbering gets off by one:
Screen Shot 2020-06-24 at 5 34 12 PM
Is this something that can be fixed within this PR as well?

@marianunez
Copy link
Contributor Author

marianunez commented Jun 25, 2020 •
edited
Loading

Is this something that can be fixed within this PR as well?

@prapti could we file this as a separate issue? This issue is not related with this PR and it's currently happening in master as well.

To fix this we would not only have to rely on the amount of posts but parse out the numbering on the last one (that may or may not be available given that people can manually queue items without numbers).

@prapti prapti self-requested a review June 25, 2020 21:19
prapti
prapti approved these changes Jun 25, 2020
View reviewed changes
Copy link

@prapti prapti left a comment

Choose a reason for hiding this comment

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

Thank you for explaining that @marianunez! Created a new ticket https://mattermost.atlassian.net/browse/MM-26536 for the issue mentioned, and approving this one.

@prapti prapti added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Jun 25, 2020
@marianunez marianunez mentioned this pull request Jun 25, 2020
Closed
@marianunez
Copy link
Contributor Author

Created a new ticket mattermost.atlassian.net/browse/MM-26536 for the issue mentioned, and approving this one.

Thanks @prapti! I've copied it to a GH issue #47 as well, I believe integrations team are tracking them there.

@marianunez marianunez merged commit 21a0836 into master Jun 25, 2020
@marianunez marianunez deleted the GH-32 branch June 25, 2020 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jfrerich jfrerich jfrerich approved these changes

@lieut-data lieut-data lieut-data approved these changes

@prapti prapti prapti approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Milestone
v0.2.0
Development

Successfully merging this pull request may close these issues.
6 participants
@marianunez @codecov-commenter @hanzei @prapti @lieut-data @jfrerich