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.

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

Fix proposal date shown to published_at #3649

Merged
merged 7 commits into from
Jun 22, 2018

Conversation

agustibr
Copy link
Contributor

@agustibr agustibr commented Jun 12, 2018

🎩 What? Why?

Creation date is shown for Proposal in: card, detail, admin component index and when exported.
But the relevant date is the Publication date, this PR solves this issues

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry
  • Added test for proposal_m_cell_spec.rb
  • Updated test for proposals_spec.rb
  • modified form created_at to published_at:
    • proposal card_m
    • proposal author card
    • proposal admin index
    • proposal serializer (export)

📷 Screenshots (optional)

@ghost ghost assigned agustibr Jun 12, 2018
@ghost ghost added the status: WIP label Jun 12, 2018
mrcasals
mrcasals previously approved these changes Jun 12, 2018
Copy link
Contributor

@mrcasals mrcasals left a comment

Choose a reason for hiding this comment

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

Wow, I was not aware we were using it in so many places, good job @agustibr! 😄

@agustibr
Copy link
Contributor Author

Jaja, I also discovered all cases on the progress 😅

@mrcasals there's one more case, it's the currently failing test, because of the proposal preview step in the wizard, at that point there's no published_at, before it rendered the created_at value, what should it show?

  1. no creation_date_status box 🗑️
  2. a blank space ⬜
  3. the created_at value
  4. or another option

screenshot from 2018-06-12 12-21-34

screenshot from 2018-06-12 12-15-20

@mrcasals
Copy link
Contributor

@agustibr I'd remove the cell (option 1). Maybe it even makes sense to remove the whole status section, since draft proposals can't be endorsed nor commented, so that data makes no sense.

What do you think? 😄

@agustibr
Copy link
Contributor Author

@mrcasals yes I think so, but it is supossed to be a preview of the final card_m.

@decidim/product what do you think is the best option here?

@xabier
Copy link
Contributor

xabier commented Jun 18, 2018

hi, I don't have strong opinion. Simplest option 1 seems ok.

@agustibr
Copy link
Contributor Author

@xabier thanks for your response, ok I'll remove the creation_date_status box from the preview

@agustibr
Copy link
Contributor Author

@mrcasals, PR ready to merge! 😬

@mrcasals mrcasals merged commit 99e1635 into master Jun 22, 2018
@mrcasals mrcasals deleted the fix/proposal-date-published_at branch June 22, 2018 07:30
@agustibr
Copy link
Contributor Author

👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants